-
Notifications
You must be signed in to change notification settings - Fork 0
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
Cloudformation stacks End to End tests - test suite structure #13
base: develop
Are you sure you want to change the base?
Conversation
role-to-assume: ${{ secrets.AWS_E2E_ROLE }} | ||
aws-region: us-east-1 | ||
|
||
- name: Run e2e tests |
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.
Why are we having a separate workflow running ?? We can include this in PR workflow itself . Just the Run e2e tests step.
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.
This will run as part of merge workflow. Right now there are 2 separate workflows for lambda code deployment, templates push. This is a new workflow
NEW_RELIC_LICENSE_KEY: ${{ secrets.NEW_RELIC_LICENSE_KEY }} | ||
run: | | ||
cd e2e-tests/ | ||
./build-templates.sh |
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.
As it involves couple of script files, add a step or job to validate these script files. can use something like shellcheck
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.
If any of the scripts fail, the workflow step itself fails. All the required checks need to be added as part of the test files itself. There's no additional check required here
deploy_stack "$LAMBDA_TEMPLATE_BUILD_DIR/$LAMBDA_TEMPLATE" "$S3_TRIGGER_CASE" "$NEW_RELIC_LICENSE_KEY" "$NEW_RELIC_REGION" "$NEW_RELIC_ACCOUNT_ID" "false" "$S3_BUCKET_NAMES" "''" "''" | ||
validate_stack_deployment_status "$S3_TRIGGER_CASE" | ||
validate_stack_resources "$S3_TRIGGER_CASE" "$S3_BUCKET_NAME" "$S3_BUCKET_PREFIX" | ||
delete_stack "$S3_TRIGGER_CASE" |
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.
Should we delete the resources only on success of stack , so that even if there is any issue in the deployment , the resources can be used to traceback
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.
delete stack happens only of the stack is in create success state. Any other state the deletion fails and the step in workflow also fails.
BUILD_DIR="$BUILD_DIR_BASE/$BASE_NAME" | ||
|
||
sam build -u --template-file "../$TEMPLATE_FILE" --build-dir "$BUILD_DIR" | ||
sam package --s3-bucket "$S3_BUCKET" --template-file "$BUILD_DIR/template.yaml" --output-template-file "$BUILD_DIR/$TEMPLATE_FILE" |
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.
As part of cleaning up of instances, delete the build file as well from the s3 bucket which gets uploaded as part of this command
on: | ||
pull_request: | ||
branches: | ||
- main |
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.
We would like to run it before merging to main, not with every pull request. Do we have to do something like what we are doing for fluent bit e.g. having a prerelease stage to run e2e tests: https://github.com/newrelic/fluent-bit-package/blob/main/.github/workflows/prerelease.yml
--template-file "$template_file" \ | ||
--stack-name "$stack_name" \ | ||
--parameter-overrides \ | ||
LicenseKey="$license_key" \ | ||
NewRelicRegion="$new_relic_region" \ | ||
NewRelicAccountId="$new_relic_account_id" \ | ||
StoreNRLicenseKeyInSecretManager="$secret_license_key" \ | ||
S3BucketNames="$s3_bucket_names" \ | ||
LogGroupConfig="$log_group_config" \ | ||
CommonAttributes="$common_attributes" \ | ||
--capabilities CAPABILITY_IAM |
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.
Each Template expect different set of params. How are you going to use a generic deploy method for all templates?
# test case constants | ||
CLOUDWATCH_TRIGGER_CASE=cloudwatch-trigger-stack-1 | ||
|
||
validate_stack_resources() { |
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.
use more specific name may be validate_lambda_subscription_created
lambda_physical_id=$(aws cloudformation describe-stack-resources \ | ||
--stack-name "$stack_name" \ | ||
--logical-resource-id "$LAMBDA_LOGICAL_RESOURCE_ID" \ | ||
--query "StackResources[0].PhysicalResourceId" \ | ||
--output text | ||
) | ||
lambda_function_arn=$(aws lambda get-function --function-name "$lambda_physical_id" \ | ||
--query "Configuration.FunctionArn" \ | ||
--output text | ||
) |
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.
you can refactor and put in common script file.
@@ -0,0 +1,19 @@ | |||
# test resources | |||
S3_BUCKET=unified-lambda-serverless-1 |
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.
we should explictly use a name which makes it obvious that its a test bucket maybe: unified-lambda-e2e-test-templates
source config-file.cfg | ||
|
||
# test case constants | ||
S3_TRIGGER_CASE=s3-trigger-stack-1 |
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.
we can use e2e in stack name to make it clear that its related to e2e.
./lambda-cloudwatch-trigger.sh | ||
./lambda-s3-trigger.sh |
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.
Can we run these tests in parallel?
This PR addresses all the basic requirements and test suite structure to run e2e tests on cloudformation stack template deployments. Refer to test documentation here