Skip to content

Commit

Permalink
feat: support partial deploy for lambda-deployer and revisit the prot…
Browse files Browse the repository at this point in the history
…ocol for deployer response
  • Loading branch information
RonWang committed May 26, 2020
1 parent 995f84d commit 6a6f04b
Show file tree
Hide file tree
Showing 9 changed files with 164 additions and 78 deletions.
29 changes: 22 additions & 7 deletions lib/builtins/deploy-delegates/cfn-deployer/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,15 @@ function bootstrap(options, callback) {
/**
* Invoke the actual deploy logic for skill's infrastructures
* @param {Object} reporter upstream CLI status reporter
* @param {Object} options
* @param {Function} callback
* @param {Object} options request object from deploy-delegate which provides user's input and project's info
* { profile, alexaRegion, skillId, skillName, code, userConfig, deployState }
* @param {Function} callback { errorStr, responseObject }
* .errorStr deployDelegate will fail immediately with this errorStr
* .responseObject.isAllStepSuccess show if the entire process succeeds
* .responseObject.isCodeDeployed true if the code is uploaded sucessfully and no need to upload again
* .responseObject.deployState record the state of deployment back to file (including partial success)
* .responseObject.endpoint the final result endpoint response
* .responseObject.resultMessage the message to summarize current situation
*/
function invoke(reporter, options, callback) {
const { profile, alexaRegion, skillId, skillName, code, userConfig, deployState = {} } = options;
Expand Down Expand Up @@ -99,21 +106,29 @@ function invoke(reporter, options, callback) {
if (deployErr) {
return callback(_composeErrorMessage(deployErr, alexaRegion));
}
let isAllStepSuccess = false;
let isCodeDeployed = false;
const { stackId, endpointUri, reasons } = deployResult;
if (!reasons || reasons === []) {
isAllStepSuccess = true;
isCodeDeployed = true;
deployState[alexaRegion].stackId = stackId;
const invokeResponse = {
isAllStepSuccess,
isCodeDeployed,
deployState: deployState[alexaRegion],
endpoint: { uri: endpointUri },
message: _composeSuccessMessage(endpointUri, alexaRegion),
deployState: deployState[alexaRegion]
resultMessage: _composeSuccessMessage(endpointUri, alexaRegion)
};
return callback(null, invokeResponse);
}

isCodeDeployed = !!(deployState && deployState[alexaRegion] && deployState[alexaRegion].s3);
callback(null, {
reasons,
message: _composeErrorMessage(reasons, alexaRegion),
deployState: deployState[alexaRegion]
isAllStepSuccess,
isCodeDeployed,
deployState: deployState[alexaRegion],
resultMessage: _composeErrorMessage(reasons, alexaRegion)
});
});
});
Expand Down
21 changes: 17 additions & 4 deletions lib/builtins/deploy-delegates/lambda-deployer/helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -212,12 +212,25 @@ function _updateLambdaFunction(reporter, lambdaClient, options, callback) {
revisionId = codeData.RevisionId;
lambdaClient.updateFunctionConfiguration(functionName, runtime, handler, revisionId, (configErr, configData) => {
if (configErr) {
return callback(configErr);
return callback(null, {
isAllStepSuccess: false,
isCodeDeployed: true,
lambdaResponse: {
arn: codeData.FunctionArn,
revisionId: codeData.RevisionId,
lastModified: codeData.LastModified
},
resultMessage: configErr
});
}
callback(null, {
arn: configData.FunctionArn,
lastModified: configData.LastModified,
revisionId: configData.RevisionId
isAllStepSuccess: true,
isCodeDeployed: true,
lambdaResponse: {
arn: configData.FunctionArn,
lastModified: configData.LastModified,
revisionId: configData.RevisionId
}
});
});
});
Expand Down
37 changes: 23 additions & 14 deletions lib/builtins/deploy-delegates/lambda-deployer/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,25 +76,34 @@ function invoke(reporter, options, callback) {
currentRegionDeployState
};
helper.deployLambdaFunction(reporter, deployLambdaConfig, (lambdaErr, lambdaResult) => {
// 1.fatal error happens in Lambda deployment and nothing need to record additionally
if (lambdaErr) {
const errMessage = `The lambda deploy failed for Alexa region "${alexaRegion}": ${lambdaErr}`;
const reasons = [errMessage];
return callback(null, {
reasons,
message: errMessage,
deployState: deployState[alexaRegion]
isAllStepSuccess: false,
isCodeDeployed: false,
deployState: deployState[alexaRegion],
resultMessage: `The lambda deploy failed for Alexa region "${alexaRegion}": ${lambdaErr}`
});
}
const { arn, lastModified, revisionId } = lambdaResult;
deployState[alexaRegion].lambda = {
arn,
lastModified,
revisionId
};
const { isAllStepSuccess, isCodeDeployed, lambdaResponse = {} } = lambdaResult;
deployState[alexaRegion].lambda = lambdaResponse;
const { arn } = lambdaResponse;
// 2.full successs in Lambda deploy
if (isAllStepSuccess) {
return callback(null, {
isAllStepSuccess,
isCodeDeployed,
deployState: deployState[alexaRegion],
endpoint: { uri: arn },
resultMessage: `The lambda deploy succeeded for Alexa region "${alexaRegion}" with output Lambda ARN: ${arn}.`
});
}
// 3.partial success in Lambda deploy (like only LambdaCode is deployed)
return callback(null, {
endpoint: { uri: arn },
message: `The lambda deploy succeeded for Alexa region "${alexaRegion}" with output Lambda ARN: ${arn}.`,
deployState: deployState[alexaRegion]
isAllStepSuccess,
isCodeDeployed,
deployState: deployState[alexaRegion],
resultMessage: `The lambda deploy failed for Alexa region "${alexaRegion}": ${lambdaResult.resultMessage}`
});
});
});
Expand Down
20 changes: 6 additions & 14 deletions lib/controllers/skill-infrastructure-controller/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,28 +202,20 @@ module.exports = class SkillInfrastructureController {
if (invokeErr) {
return callback(invokeErr);
}
const { deployState, message, reasons } = invokeResult;
// track the current hash when deploy state contains s3 result
if (deployState && deployState.s3) { // TODO how come there is an S3 here...
const { isAllStepSuccess, isCodeDeployed } = invokeResult;
// track the current hash if isCodeDeployed
if (isCodeDeployed) {
invokeResult.lastDeployHash = currentHash;
}
if (reasons) {
// when "reasons" exist, regional deployment fails and need to callback with an error object with message and the deploy context
const context = {
deployState: invokeResult.deployState,
lastDeployHash: invokeResult.lastDeployHash
};
callback({ message, context });
} else {
callback(null, invokeResult);
}
// pass back result based on if isAllStepSuccess, pass result as error if not all steps succeed
callback(isAllStepSuccess ? null : invokeResult, isAllStepSuccess ? invokeResult : undefined);
});
});
}

/**
* Update the the ask resources config and the deploy state.
* @param {Object} rawDeployResult deploy result from invoke: { $region: { endpoint: { url }, lastDeployHash, deployState } }
* @param {Object} rawDeployResult deploy result from invoke: { $region: deploy-delegate's response }
*/
_updateResourcesConfig(rawDeployResult) {
const newDeployState = {};
Expand Down
8 changes: 3 additions & 5 deletions lib/view/multi-tasks-view.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,9 @@ class ListrReactiveTask {
this._eventEmitter.on('error', (error) => {
if (error && typeof error === 'string') {
subscriber.error(error);
} else if (error && error.message) {
subscriber.error(error.message);
if (error.context) {
ctx[this.taskId] = error.context;
}
} else if (error && error.resultMessage) {
subscriber.error(error.resultMessage);
ctx[this.taskId] = error;
}
});
this._eventEmitter.on('title', (title) => {
Expand Down
26 changes: 21 additions & 5 deletions test/unit/builtins/lambda-deployer/helper-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -390,9 +390,19 @@ ${TEST_REMOTE_REVISION_ID}, but found ${TEST_LOCAL_REVISION_ID}. Please solve th
sinon.stub(LambdaClient.prototype, 'updateFunctionCode').callsArgWith(3, null, { RevisionId: TEST_REVISION_ID });
sinon.stub(LambdaClient.prototype, 'updateFunctionConfiguration').callsArgWith(4, TEST_UPDATE_CONGIF_ERROR);
// call
helper.deployLambdaFunction(REPORTER, TEST_UPDATE_OPTIONS, (err) => {
helper.deployLambdaFunction(REPORTER, TEST_UPDATE_OPTIONS, (err, res) => {
// verify
expect(err).equal(TEST_UPDATE_CONGIF_ERROR);
expect(err).equal(null);
expect(res).deep.equal({
isAllStepSuccess: false,
isCodeDeployed: true,
lambdaResponse: {
arn: undefined,
lastModified: undefined,
revisionId: TEST_REVISION_ID
},
resultMessage: TEST_UPDATE_CONGIF_ERROR
});
done();
});
});
Expand All @@ -411,9 +421,15 @@ ${TEST_REMOTE_REVISION_ID}, but found ${TEST_LOCAL_REVISION_ID}. Please solve th
// call
helper.deployLambdaFunction(REPORTER, TEST_UPDATE_OPTIONS, (err, data) => {
// verify
expect(data.arn).equal(TEST_LAMBDA_ARN);
expect(data.lastModified).equal(TEST_LAST_MODIFIED);
expect(data.revisionId).equal(TEST_REVISION_ID);
expect(data).deep.equal({
isAllStepSuccess: true,
isCodeDeployed: true,
lambdaResponse: {
arn: TEST_LAMBDA_ARN,
lastModified: TEST_LAST_MODIFIED,
revisionId: TEST_REVISION_ID
}
});
done();
});
});
Expand Down
50 changes: 38 additions & 12 deletions test/unit/builtins/lambda-deployer/index-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ describe('Builtins test - lambda-deployer index.js test', () => {
const TEST_OPTIONS_WITHOUT_PROFILE = {
profile: null,
};
const TEST_ERROR = `Profile [${NULL_PROFILE}] doesn't have AWS profile linked to it. Please run "ask configure" to re-configure your porfile.`;
const TEST_ERROR = `Profile [${NULL_PROFILE}] doesn't have AWS profile linked to it. \
Please run "ask configure" to re-configure your porfile.`;
sinon.stub(awsUtil, 'getAWSProfile').withArgs(NULL_PROFILE).returns(NULL_PROFILE);
// call
lambdaDeployer.invoke(REPORTER, TEST_OPTIONS_WITHOUT_PROFILE, (err) => {
Expand Down Expand Up @@ -135,9 +136,24 @@ describe('Builtins test - lambda-deployer index.js test', () => {
// call
lambdaDeployer.invoke(REPORTER, TEST_OPTIONS, (err, data) => {
// verify
expect(data.reasons).to.be.an.instanceOf(Array);
expect(data.reasons.length).equal(1);
expect(data.message).equal(TEST_ERROR_MESSAGE_RESPONSE);
expect(data.resultMessage).equal(TEST_ERROR_MESSAGE_RESPONSE);
expect(data.deployState.iamRole).equal(TEST_IAM_ROLE_ARN);
done();
});
});

it('| deploy IAM role passes, deploy Lambda code configuration fails, expect IAM role arn, revisionId and a message returned', (done) => {
// setup
const TEST_ERROR = 'LambdaFunction error message';
const TEST_ERROR_MESSAGE_RESPONSE = `The lambda deploy failed for Alexa region "default": ${TEST_ERROR}`;
sinon.stub(awsUtil, 'getAWSProfile').withArgs(TEST_PROFILE).returns(TEST_PROFILE);
sinon.stub(helper, 'validateLambdaDeployState').callsArgWith(4, null, TEST_VALIDATED_DEPLOY_STATE);
sinon.stub(helper, 'deployIAMRole').callsArgWith(6, null, TEST_IAM_ROLE_ARN);
sinon.stub(helper, 'deployLambdaFunction').callsArgWith(2, TEST_ERROR);
// call
lambdaDeployer.invoke(REPORTER, TEST_OPTIONS, (err, data) => {
// verify
expect(data.resultMessage).equal(TEST_ERROR_MESSAGE_RESPONSE);
expect(data.deployState.iamRole).equal(TEST_IAM_ROLE_ARN);
done();
});
Expand All @@ -150,26 +166,32 @@ describe('Builtins test - lambda-deployer index.js test', () => {
const REVISION_ID = '1';
const TEST_SUCCESS_RESPONSE = `The lambda deploy succeeded for Alexa region "${TEST_ALEXA_REGION_DEFAULT}" \
with output Lambda ARN: ${LAMBDA_ARN}.`;
const LAMDBA_RESULT = {
const LAMDBA_RESPONSE = {
arn: LAMBDA_ARN,
lastModified: LAST_MODIFIED,
revisionId: REVISION_ID
};
const TEST_LAMBDA_RESULT = {
isAllStepSuccess: true,
isCodeDeployed: true,
lambdaResponse: LAMDBA_RESPONSE
};
sinon.stub(awsUtil, 'getAWSProfile').withArgs(TEST_PROFILE).returns(TEST_PROFILE);
sinon.stub(helper, 'validateLambdaDeployState').callsArgWith(4, null, TEST_VALIDATED_DEPLOY_STATE);
sinon.stub(helper, 'deployIAMRole').callsArgWith(6, null, TEST_IAM_ROLE_ARN);
sinon.stub(helper, 'deployLambdaFunction').callsArgWith(2, null, LAMDBA_RESULT);
sinon.stub(helper, 'deployLambdaFunction').callsArgWith(2, null, TEST_LAMBDA_RESULT);
// call
lambdaDeployer.invoke(REPORTER, TEST_OPTIONS, (err, res) => {
// verify
expect(res.endpoint.uri).equal(LAMBDA_ARN);
expect(res.deployState.iamRole).equal(TEST_IAM_ROLE_ARN);
expect(res.message).equal(TEST_SUCCESS_RESPONSE);
expect(res.deployState.lambda).deep.equal(LAMDBA_RESPONSE);
expect(res.resultMessage).equal(TEST_SUCCESS_RESPONSE);
expect(err).equal(null);
done();
});
});

it('| alexaRegion is default, userConfig awsRegion is set, expect awsRegion is retrieved correctly.', (done) => {
// setup
const USER_CONFIG_AWS_REGION = 'sa-east-1';
Expand All @@ -184,11 +206,15 @@ with output Lambda ARN: ${LAMBDA_ARN}.`;
},
deployState: {}
};
const LAMDBA_RESULT = {};
const TEST_LAMBDA_RESULT = {
isAllStepSuccess: true,
isCodeDeployed: true,
lambdaResponse: {}
};
sinon.stub(awsUtil, 'getAWSProfile').withArgs(TEST_PROFILE).returns(TEST_PROFILE);
sinon.stub(helper, 'validateLambdaDeployState').callsArgWith(4, null, TEST_VALIDATED_DEPLOY_STATE);
sinon.stub(helper, 'deployIAMRole').callsArgWith(6, null, TEST_IAM_ROLE_ARN);
sinon.stub(helper, 'deployLambdaFunction').callsArgWith(2, null, LAMDBA_RESULT);
sinon.stub(helper, 'deployLambdaFunction').callsArgWith(2, null, TEST_LAMBDA_RESULT);
// call
lambdaDeployer.invoke(REPORTER, TEST_OPTIONS_WITHOUT_AWS_REGION, (err) => {
// verify
Expand All @@ -198,7 +224,7 @@ with output Lambda ARN: ${LAMBDA_ARN}.`;
done();
});
});

it('| alexaRegion is default, userConfig awsRegion is NOT set, expect awsRegion is set based on Alexa and AWS region map.', (done) => {
// setup
const MAPPING_ALEXA_DEFAULT_AWS_REGION = 'us-east-1';
Expand All @@ -225,7 +251,7 @@ with output Lambda ARN: ${LAMBDA_ARN}.`;
done();
});
});

it('| alexaRegion is not default, userConfig regionalOverrides awsRegion is set, expect awsRegion is retrieved correctly.', (done) => {
// setup
const TEST_ALEXA_REGION_EU = 'EU';
Expand Down
Loading

0 comments on commit 6a6f04b

Please sign in to comment.