You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2021/02/17 21:46:35 UTC
[GitHub] [superset] robdiciuccio opened a new pull request #13189: build: Ephemeral environments for PRs via slash command
robdiciuccio opened a new pull request #13189:
URL: https://github.com/apache/superset/pull/13189
### SUMMARY
Github Actions that provide the ability to create an ephemeral environment running a PR's latest merge commit via a command executed via PR comment. The Docker build is run in a virtual environment created via [AWS ECS](https://aws.amazon.com/ecs/), using the [Fargate](https://aws.amazon.com/fargate/) container runtime. This functionality is sponsored by @preset-io.
- Adds an additional Docker build target (**ci**) which includes additional scripts to initialize and run the container instance
- Builds the Docker **ci** target on `pull_request` events (utilizing the cache from previous `docker build` runs) and saves the build as a [workflow artifact](https://docs.github.com/en/actions/guides/storing-workflow-data-as-artifacts).
- Adds a **Push ephmereral env image** (`workflow_run`) workflow to tag and push the Docker **ci** image to [AWS ECR](https://aws.amazon.com/ecr/).
- Adds an **Ephemeral env workflow** (`issue_comment`) workflow to evaluate PR comments for the `/testenv up` or `/testenv down` commands and deploy ECS services accordingly.
- Adds new secrets to the repo: `AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY`. These credentials correspond to an AWS IAM user with appropriate ECS privileges. An Apache INFRA ticket is pending to add these secrets to the repo.
### Running
- When a PR is opened or updated, a Docker **ci** build of the merge commit will be saved and uploaded to ECR.
- Posting a comment on the PR containing `/testenv up` (and nothing else) will trigger the ephemeral environment creation or update. _Note: this functionality is currently restricted to members of the **apache** Github organization._
- When the environment is created, a comment will be posted to the PR with the relevant details:
<img width="926" alt="Screen Shot 2021-02-17 at 12 34 15 PM" src="https://user-images.githubusercontent.com/296227/108267592-4080b100-7120-11eb-8de6-35f6e452af49.png">
- Note that the environment will take several minutes to become available, as all DB migrations and example data loading is performed upon startup.
- If the environment fails to startup, for example, if the Docker image is not yet built and pushed to ECR, a comment will also be posted to the PR:
<img width="925" alt="Screen Shot 2021-02-17 at 1 04 21 PM" src="https://user-images.githubusercontent.com/296227/108267928-b08f3700-7120-11eb-8c9b-99ed915ce095.png">
### Security
The enclosed workflows follow [Github's security guidelines](https://securitylab.github.com/research/github-actions-preventing-pwn-requests) for builds. Docker images are built via the read-only `pull_request` event, preventing malicious code from accessing tokens with repo write access. `workflow_run` and `issue_comment` privileged workflows are limited to uploading and interacting with the AWS API, not building or running any of the untrusted code.
### TEST PLAN
- Follow the procedure in [Running](#Running), above. _Note that the `workflow_run` and `issue_comment` workflows will not run until they are merged to `master`._
- These workflows have been tested pretty extensively on [my fork of Superset](https://github.com/robdiciuccio/incubator-superset/pulls).
### TODO (in future PRs)
- Add handler for `/testenv down` command to manually shutdown environments
- Automate shutting down server resources after ~48h
- Shutdown and cleanup on PR close
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on pull request #13189: build: Ephemeral environments for PRs via slash command
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #13189:
URL: https://github.com/apache/superset/pull/13189#issuecomment-780880918
Super exciting for this!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on a change in pull request #13189: build: Ephemeral environments for PRs via slash command
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #13189:
URL: https://github.com/apache/superset/pull/13189#discussion_r578009821
##########
File path: .github/workflows/docker.yml
##########
@@ -24,3 +24,19 @@ jobs:
DOCKERHUB_TOKEN: ${{ secrets.DOCKERHUB_TOKEN }}
run: |
.github/workflows/docker_build_push.sh
+
+ - name: Build ephemeral env image
+ if: github.event_name == 'pull_request'
+ run: |
+ mkdir -p ./build
+ echo ${{ github.sha }} > ./build/SHA
+ echo ${{ github.event.pull_request.number }} > ./build/PR-NUM
+ docker build --target ci -t ${{ github.sha }} -t "pr-${{ github.event.pull_request.number }}" .
+ docker save ${{ github.sha }} | gzip > ./build/${{ github.sha }}.tar.gz
Review comment:
Will it make sense to save the already built image from previously step and rely on file names to get both SHA and PR number?
##########
File path: .github/workflows/ephemeral-env.yml
##########
@@ -0,0 +1,174 @@
+name: Ephemeral env workflow
+
+on:
+ issue_comment:
+ types: [created]
+
+jobs:
+ ephemeral_env_comment:
+ if: github.event.issue.pull_request
+ name: Evaluate ephemeral env comment trigger (/testenv)
+ runs-on: ubuntu-latest
+ outputs:
+ slash-command: ${{ steps.eval-body.outputs.result }}
+
+ steps:
+ - name: Debug
+ run: |
+ echo "Comment on PR #${{ github.event.issue.number }} by ${{ github.event.issue.user.login }}, ${{ github.event.comment.author_association }}"
+ echo "Comment body: ${{ github.event.comment.body }}"
+
+ - name: Eval comment body for /testenv slash command
+ uses: actions/github-script@v3
+ id: eval-body
+ with:
+ result-encoding: string
+ script: |
+ const pattern = /^\/testenv (up|down)/
+ const result = pattern.exec(context.payload.comment.body)
+ return result === null ? 'noop' : result[1]
+
+ - name: Limit to committers
+ if: >
+ steps.eval-body.outputs.result != 'noop' &&
+ github.event.comment.author_association != 'MEMBER' &&
+ github.event.comment.author_association != 'OWNER'
+ uses: actions/github-script@v3
+ with:
+ github-token: ${{secrets.GITHUB_TOKEN}}
+ script: |
+ const errMsg = 'Ephemeral environment creation is currently limited to committers.'
Review comment:
Maybe add a `@comment_author` here 😉
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] junlincc commented on pull request #13189: build: Ephemeral environments for PRs via slash command
Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #13189:
URL: https://github.com/apache/superset/pull/13189#issuecomment-780922255
Thanks for setting this up; it's going to improve the manual testing process vastly. 🙂
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud edited a comment on pull request #13189: build: Ephemeral environments for PRs via slash command
Posted by GitBox <gi...@apache.org>.
ktmud edited a comment on pull request #13189:
URL: https://github.com/apache/superset/pull/13189#issuecomment-780880918
Super excited for this! Thanks for setting this up and the sponsorship!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] robdiciuccio commented on pull request #13189: build: Ephemeral environments for PRs via slash command
Posted by GitBox <gi...@apache.org>.
robdiciuccio commented on pull request #13189:
URL: https://github.com/apache/superset/pull/13189#issuecomment-782442646
Apache INFRA ticket: https://issues.apache.org/jira/browse/INFRA-21442
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud edited a comment on pull request #13189: build: Ephemeral environments for PRs via slash command
Posted by GitBox <gi...@apache.org>.
ktmud edited a comment on pull request #13189:
URL: https://github.com/apache/superset/pull/13189#issuecomment-780880918
Super exciting for this! Thanks for setting this up and the sponsorship!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] robdiciuccio commented on pull request #13189: build: Ephemeral environments for PRs via slash command
Posted by GitBox <gi...@apache.org>.
robdiciuccio commented on pull request #13189:
URL: https://github.com/apache/superset/pull/13189#issuecomment-781490270
> Since issue_comment event also has access to all the base repo secrets, would it make sense to bypass the workflow_run step and upload artifacts to ECR in the issue_comment workflow, too?
Ideally, yes, but there's no way (that I've found) for an `issue_comment` event to reference workflow runs for a PR, making it impossible to retrieve the uploaded build artifacts.
> The built docker image is kind of large, too. I'm wondering whether it's possible to use the latest released apache/superset:master as the base image and build a new image just for ECR? (I'd imagine the saved docker build will only contain the new layers with pip package, python file, and static assets overrides.)
Not sure how this would work in a PR context, as the standard docker builds are not pushed to Dockerhub for PRs from forked repos (no secrets access). The current CI (ephemeral env) build is leveraging the cache from the current Docker build process, but agreed that the image size is large. I'm working on getting the overall image size down, but on first pass there's not a whole lot of wasted space here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] junlincc commented on pull request #13189: build: Ephemeral environments for PRs via slash command
Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #13189:
URL: https://github.com/apache/superset/pull/13189#issuecomment-785300651
@robdiciuccio thanks again for improving the process. Do you mind adding a non-technical user friendly tutorial, so our designers and PMs can all benefit from it? 🙏
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] robdiciuccio merged pull request #13189: build: Ephemeral environments for PRs via slash command
Posted by GitBox <gi...@apache.org>.
robdiciuccio merged pull request #13189:
URL: https://github.com/apache/superset/pull/13189
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] robdiciuccio commented on pull request #13189: build: Ephemeral environments for PRs via slash command
Posted by GitBox <gi...@apache.org>.
robdiciuccio commented on pull request #13189:
URL: https://github.com/apache/superset/pull/13189#issuecomment-785294898
Secrets have been added by Apache infra.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on pull request #13189: build: Ephemeral environments for PRs via slash command
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #13189:
URL: https://github.com/apache/superset/pull/13189#issuecomment-781702729
> Ideally, yes, but there's no way (that I've found) for an `issue_comment` event to reference workflow runs for a PR, making it impossible to retrieve the uploaded build artifacts.
>
You can potentially get the branch name of the PR first, then find the workflow runs by branch name. I've done that in the `cancel_github_workflow` script:
https://github.com/apache/superset/blob/2456a2eb99d0abdeec12a5e5c889d418f27e2c74/scripts/cancel_github_workflows.py#L176-L183
> Not sure how this would work in a PR context, as the standard docker builds are not pushed to Dockerhub for PRs from forked repos (no secrets access). The current CI (ephemeral env) build is leveraging the cache from the current Docker build process, but agreed that the image size is large. I'm working on getting the overall image size down, but on first pass there's not a whole lot of wasted space here.
I'm not familiar with Docker, but I was imaging if you have a `Dockerfile` like this:
```dockerfile
FROM apache/superset:master
# copy latest file contents
...
# re-install dependencies
...
# rebuild assets
...
```
The saved docker image will contain only the additional layers on top of `apache/superset`, so the image you uploaded to Amazon would be smaller than building Superset from scratch.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] robdiciuccio commented on pull request #13189: build: Ephemeral environments for PRs via slash command
Posted by GitBox <gi...@apache.org>.
robdiciuccio commented on pull request #13189:
URL: https://github.com/apache/superset/pull/13189#issuecomment-781729851
> The saved docker image will contain only the additional layers on top of apache/superset, so the image you uploaded to Amazon would be smaller than building Superset from scratch.
Reinstalling dependencies and rebuilding assets would add significant time to the build of the CI image. Running webpack is the most expensive operation in the build.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] robdiciuccio commented on a change in pull request #13189: build: Ephemeral environments for PRs via slash command
Posted by GitBox <gi...@apache.org>.
robdiciuccio commented on a change in pull request #13189:
URL: https://github.com/apache/superset/pull/13189#discussion_r578592057
##########
File path: .github/workflows/docker.yml
##########
@@ -24,3 +24,19 @@ jobs:
DOCKERHUB_TOKEN: ${{ secrets.DOCKERHUB_TOKEN }}
run: |
.github/workflows/docker_build_push.sh
+
+ - name: Build ephemeral env image
+ if: github.event_name == 'pull_request'
+ run: |
+ mkdir -p ./build
+ echo ${{ github.sha }} > ./build/SHA
+ echo ${{ github.event.pull_request.number }} > ./build/PR-NUM
+ docker build --target ci -t ${{ github.sha }} -t "pr-${{ github.event.pull_request.number }}" .
+ docker save ${{ github.sha }} | gzip > ./build/${{ github.sha }}.tar.gz
Review comment:
Since the `--target ci` build here is already using the cached layers from the previous builds, there's not much of a gain in using those images directly. We could embed both the PR num and SHA in the filename, but this felt a bit cleaner, and wouldn't save any measurable time in the `workflow_run` step.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] robdiciuccio commented on pull request #13189: build: Ephemeral environments for PRs via slash command
Posted by GitBox <gi...@apache.org>.
robdiciuccio commented on pull request #13189:
URL: https://github.com/apache/superset/pull/13189#issuecomment-785304589
@junlincc yes, I'll be adding some documentation once the feature has gone through some testing on the repo (merging this PR is required for testing due to the security model of GitHub Action).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org