Skip to content

Commit

Permalink
ACNA-858 - support for app hook long-running processes via aio-run-de…
Browse files Browse the repository at this point in the history
…tached (#301)
  • Loading branch information
shazron authored Oct 21, 2020
1 parent 88dbbc9 commit db51485
Show file tree
Hide file tree
Showing 12 changed files with 213 additions and 63 deletions.
8 changes: 4 additions & 4 deletions src/commands/app/deploy.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class Deploy extends BaseCommand {
try {
await runPackageScript('pre-app-build')
} catch (err) {
// this is assumed to be a missing script error
this.log(err)
}

if (!flags['skip-actions']) {
Expand Down Expand Up @@ -74,7 +74,7 @@ class Deploy extends BaseCommand {
try {
await runPackageScript('post-app-build')
} catch (err) {
// this is assumed to be a missing script error
this.log(err)
}
}

Expand All @@ -86,7 +86,7 @@ class Deploy extends BaseCommand {
try {
await runPackageScript('pre-app-deploy')
} catch (err) {
// this is assumed to be a missing script error
this.log(err)
}
if (!flags['skip-actions']) {
if (fs.existsSync('manifest.yml')) {
Expand Down Expand Up @@ -134,7 +134,7 @@ class Deploy extends BaseCommand {
try {
await runPackageScript('post-app-deploy')
} catch (err) {
// this is assumed to be a missing script error
this.log(err)
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/commands/app/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class Run extends BaseCommand {
try {
await runPackageScript('pre-app-run')
} catch (err) {
// this is assumed to be a missing script error
this.log(err)
}

// check if there are certificates available, and generate them if not ...
Expand All @@ -81,7 +81,7 @@ class Run extends BaseCommand {
try {
await runPackageScript('post-app-run')
} catch (err) {
// this is assumed to be a missing script error
this.log(err)
}
if (frontendUrl) {
this.log()
Expand Down
38 changes: 35 additions & 3 deletions src/lib/app-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,31 @@ async function runPackageScript (scriptName, dir, cmdArgs = []) {
aioLogger.debug(`running npm run-script ${scriptName} in dir: ${dir}`)
const pkg = await fs.readJSON(path.join(dir, 'package.json'))
if (pkg && pkg.scripts && pkg.scripts[scriptName]) {
return execa('npm', ['run-script', scriptName].concat(cmdArgs), { cwd: dir, stdio: 'inherit' })

This comment has been minimized.

Copy link
@purplecabbage

purplecabbage Oct 30, 2020

Member

This broke support for chaining commands, and other command separators.
Reported in #315

const command = pkg.scripts[scriptName].split(' ')
const child = execa(command[0], command.slice(1).concat(cmdArgs), {
stdio: ['inherit', 'inherit', 'inherit', 'ipc'],
cwd: dir,
preferLocal: true
})
// handle IPC from possible aio-run-detached script
child.on('message', message => {
if (message.type === 'long-running-process') {
const { pid, logs } = message.data
aioLogger.debug(`Found ${scriptName} event hook long running process (pid: ${pid}). Registering for SIGTERM`)
aioLogger.debug(`Log locations for ${scriptName} event hook long-running process (stdout: ${logs.stdout} stderr: ${logs.stderr})`)
process.on('exit', () => {
try {
aioLogger.debug(`Killing ${scriptName} event hook long-running process (pid: ${pid})`)
process.kill(pid, 'SIGTERM')
} catch (_) {
// do nothing if pid not found
}
})
}
})
return child
} else {
throw new Error(`${dir} does not contain a package.json or it does not contain a script named ${scriptName}`)
aioLogger.debug(`${dir} does not contain a package.json or it does not contain a script named ${scriptName}`)
}
}

Expand Down Expand Up @@ -85,6 +107,7 @@ async function getCliInfo () {
return { accessToken, env }
}

/** @private */
function getActionUrls (config, isRemoteDev = false, isLocalDev = false) {
// set action urls
// action urls {name: url}, if !LocalDev subdomain uses namespace
Expand Down Expand Up @@ -164,6 +187,7 @@ function writeConfig (file, config) {
)
}

/** @private */
async function isDockerRunning () {
// todo more checks
const args = ['info']
Expand All @@ -176,6 +200,7 @@ async function isDockerRunning () {
return false
}

/** @private */
async function hasDockerCLI () {
// todo check min version
try {
Expand All @@ -188,6 +213,7 @@ async function hasDockerCLI () {
return false
}

/** @private */
async function hasJavaCLI () {
// todo check min version
try {
Expand All @@ -211,6 +237,7 @@ async function hasJavaCLI () {
// return false
// }

/** @private */
async function downloadOWJar (url, outFile) {
aioLogger.debug(`downloadOWJar - url: ${url} outFile: ${outFile}`)
let response
Expand All @@ -234,19 +261,21 @@ async function downloadOWJar (url, outFile) {
})
})
}

/** @private */
async function runOpenWhiskJar (jarFile, runtimeConfigFile, apihost, waitInitTime, waitPeriodTime, timeout, /* istanbul ignore next */ execaOptions = {}) {
aioLogger.debug(`runOpenWhiskJar - jarFile: ${jarFile} runtimeConfigFile ${runtimeConfigFile} apihost: ${apihost} waitInitTime: ${waitInitTime} waitPeriodTime: ${waitPeriodTime} timeout: ${timeout}`)
const proc = execa('java', ['-jar', '-Dwhisk.concurrency-limit.max=10', jarFile, '-m', runtimeConfigFile, '--no-ui'], execaOptions)
await waitForOpenWhiskReadiness(apihost, waitInitTime, waitPeriodTime, timeout)
// must wrap in an object as execa return value is awaitable
return { proc }

/** @private */
async function waitForOpenWhiskReadiness (host, initialWait, period, timeout) {
const endTime = Date.now() + timeout
await waitFor(initialWait)
await _waitForOpenWhiskReadiness(host, endTime)

/** @private */
async function _waitForOpenWhiskReadiness (host, endTime) {
if (Date.now() > endTime) {
throw new Error(`local openwhisk stack startup timed out: ${timeout}ms`)
Expand All @@ -263,12 +292,15 @@ async function runOpenWhiskJar (jarFile, runtimeConfigFile, apihost, waitInitTim
return _waitForOpenWhiskReadiness(host, endTime)
}
}

/** @private */
function waitFor (t) {
return new Promise(resolve => setTimeout(resolve, t))
}
}
}

/** @private */
function saveAndReplaceDotEnvCredentials (dotenvFile, saveFile, apihost, namespace, auth) {
if (fs.existsSync(saveFile)) throw new Error(`cannot save .env, please make sure to restore and delete ${saveFile}`) // todo make saveFile relative
fs.moveSync(dotenvFile, saveFile)
Expand Down
80 changes: 41 additions & 39 deletions src/lib/config-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,45 +29,46 @@ const {
AIO_CONFIG_IMS_ORG_ID
} = require('./defaults')

/** loading config returns following object (this config is internal, not user facing):
{
app: {
name,
version,
hasFrontend
},
ow: {
apihost,
apiversion,
auth,
namespace,
package
},
s3: {
creds || tvmUrl,
credsCacheFile,
folder,
},
web: {
src,
injectedConfig,
distDev,
distProd,
},
manifest: {
full,
package,
packagePlaceholder,
src,
},
actions: {
src,
dist,
remote,
urls
}
}
*/
/**
* loading config returns following object (this config is internal, not user facing):
* {
* app: {
* name,
* version,
* hasFrontend
* },
* ow: {
* apihost,
* apiversion,
* auth,
* namespace,
* package
* },
* s3: {
* creds || tvmUrl,
* credsCacheFile,
* folder,
* },
* web: {
* src,
* injectedConfig,
* distDev,
* distProd,
* },
* manifest: {
* full,
* package,
* packagePlaceholder,
* src,
* },
* actions: {
* src,
* dist,
* remote,
* urls
* }
* }
*/

module.exports = () => {
// init internal config
Expand Down Expand Up @@ -165,6 +166,7 @@ module.exports = () => {
return config
}

/** @private */
function getModuleName (packagejson) {
if (packagejson && packagejson.name) {
// turn "@company/myaction" into "myaction"
Expand Down
2 changes: 2 additions & 0 deletions src/lib/owlocal.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@ const execa = require('execa')

const OW_LOCAL_DOCKER_PORT = 3233

/** @private */
function isWindowsOrMac () {
return (
process.platform === 'win32' ||
process.platform === 'darwin'
)
}

/** @private */
function getDockerNetworkAddress () {
try {
// Docker for Windows and macOS do not allow routing to the containers via
Expand Down
6 changes: 6 additions & 0 deletions src/lib/runDev.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const fetchLogInterval = 10000
const logOptions = {}
const eventPoller = new EventPoller(fetchLogInterval)

/** @private */
async function runDev (args = [], config, options = {}, log) {
// note: args are considered perfect here because this function is only ever called by the `app run` command
let logFunc = log
Expand Down Expand Up @@ -230,6 +231,7 @@ async function runDev (args = [], config, options = {}, log) {
return frontEndUrl
}

/** @private */
async function logListener (args) {
if (!args.resources.stopFetchLogs) {
try {
Expand All @@ -245,6 +247,7 @@ async function logListener (args) {
}
}

/** @private */
async function generateVSCodeDebugConfig (devConfig, withBackend, hasFrontend, frontUrl, wskdebugProps) {
const actionConfigNames = []
let actionConfigs = []
Expand Down Expand Up @@ -329,6 +332,7 @@ async function generateVSCodeDebugConfig (devConfig, withBackend, hasFrontend, f
return debugConfig
}

/** @private */
function _getActionChangeHandler (devConfig, isLocalDev, logFunc) {
return async (filePath) => {
if (running) {
Expand All @@ -355,6 +359,7 @@ function _getActionChangeHandler (devConfig, isLocalDev, logFunc) {
}
}

/** @private */
async function _buildAndDeploy (devConfig, isLocalDev, logFunc) {
await BuildActions(devConfig)
const entities = await DeployActions(devConfig, { isLocalDev })
Expand All @@ -365,6 +370,7 @@ async function _buildAndDeploy (devConfig, isLocalDev, logFunc) {
}
}

/** @private */
async function cleanup (resources) {
if (watcher) {
aioLogger.debug('stopping action watcher...')
Expand Down
2 changes: 2 additions & 0 deletions test/__fixtures__/sample-app/actions/action-zip/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTA
OF ANY KIND, either express or implied. See the License for the specific language
governing permissions and limitations under the License.
*/

/** @private */
function main (args) {
return 'hello'
}
Expand Down
2 changes: 2 additions & 0 deletions test/__fixtures__/sample-app/actions/action.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTA
OF ANY KIND, either express or implied. See the License for the specific language
governing permissions and limitations under the License.
*/

/** @private */
function main (args) {
return 'hello'
}
Expand Down
17 changes: 17 additions & 0 deletions test/commands/app/deploy.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ beforeEach(() => {
mockWebLib.mockReset('buildWeb')
mockFS.existsSync.mockReset()
helpers.writeConfig.mockReset()
helpers.runPackageScript.mockReset()
jest.restoreAllMocks()

helpers.wrapError.mockImplementation(msg => msg)
Expand Down Expand Up @@ -387,4 +388,20 @@ describe('run', () => {
expect(helpers.writeConfig).toHaveBeenCalledWith('sdf', { a: 'a' })
expect(mockWebLib.deployWeb).toHaveBeenCalledTimes(1)
})

test('build & deploy (app hooks missing)', async () => {
mockFS.existsSync.mockReturnValue(true)
helpers.runPackageScript
.mockRejectedValueOnce('error-1')
.mockRejectedValueOnce('error-2')
.mockRejectedValueOnce('error-3')
.mockRejectedValueOnce('error-4')

await command.run()
expect(command.error).toHaveBeenCalledTimes(0)
expect(command.log).toHaveBeenCalledWith('error-1')
expect(command.log).toHaveBeenCalledWith('error-2')
expect(command.log).toHaveBeenCalledWith('error-3')
expect(command.log).toHaveBeenCalledWith('error-4')
})
})
Loading

0 comments on commit db51485

Please sign in to comment.