build: Ephemeral environments for PRs via slash command #13189

robdiciuccio · 2021-02-17T21:46:27Z

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, using the 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.
Adds a Push ephmereral env image (workflow_run) workflow to tag and push the Docker ci image to AWS 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:

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:

Security

The enclosed workflows follow Github's security guidelines 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, 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.

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
Add support for celery and async jobs
Add support for feature flag and configuration overrides

ADDITIONAL INFORMATION

ktmud · 2021-02-17T21:56:49Z

Super excited for this! Thanks for setting this up and the sponsorship!

junlincc · 2021-02-17T23:29:13Z

Thanks for setting this up; it's going to improve the manual testing process vastly. 🙂

ktmud

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?

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.)

ktmud · 2021-02-17T23:11:48Z

.github/workflows/docker.yml

+          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


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?

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.

ktmud · 2021-02-18T01:29:25Z

.github/workflows/ephemeral-env.yml

+      with:
+        github-token: ${{secrets.GITHUB_TOKEN}}
+        script: |
+          const errMsg = 'Ephemeral environment creation is currently limited to committers.'


Maybe add a @comment_author here 😉

robdiciuccio · 2021-02-18T17:00:37Z

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.

ktmud · 2021-02-18T23:27:53Z

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:

superset/scripts/cancel_github_workflows.py

Lines 176 to 183 in 2456a2e

    
           if pr: 
        
               runs = get_runs( 
        
                   repo, 
        
                   statuses=statuses, 
        
                   events=event, 
        
                   branch=pr["head"]["ref"], 
        
                   user=pr["user"]["login"], 
        
               )

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:

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.

robdiciuccio · 2021-02-19T00:33:59Z

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.

robdiciuccio · 2021-02-19T23:16:37Z

Apache INFRA ticket: https://issues.apache.org/jira/browse/INFRA-21442

craig-rueda

LGTM

robdiciuccio · 2021-02-24T18:49:40Z

Secrets have been added by Apache infra.

junlincc · 2021-02-24T18:58:22Z

@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? 🙏

robdiciuccio · 2021-02-24T19:04:41Z

@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).

* First pass at ephemeral env, new Docker ci target * Add service checks, get public IP * Separate issue_comment and workflow_run jobs * Refactor workflows * Adjust comment author association * Checkout code * Fix image name, manage service desired task count * Use merge commit sha * Fix IP output, add failure comment * Refactor comment parsing & env spinup * Check container image publish status * Parse AWS account ID from registry URL * Use PR number rather than variable merge commit SHA for image tag * Fix docker push conditional * Push multiple tags to ECR * Fix comment author check * Refactor comment body check * Provision service with active task to get correct IP * /testenv up * Add @mentions to PR comments, env var cleanup

robdiciuccio added 19 commits February 4, 2021 15:47

First pass at ephemeral env, new Docker ci target

32dbdfe

Add service checks, get public IP

71607e0

Separate issue_comment and workflow_run jobs

e39bed5

Refactor workflows

37468b3

Adjust comment author association

18ca7c5

Checkout code

6614cd7

Fix image name, manage service desired task count

034fa3a

Use merge commit sha

7af1a6b

Fix IP output, add failure comment

d0b8c11

Refactor comment parsing & env spinup

5a07e42

Check container image publish status

aa3bfd2

Parse AWS account ID from registry URL

4069f7b

Use PR number rather than variable merge commit SHA for image tag

20d53d0

Fix docker push conditional

705ad06

Push multiple tags to ECR

63bc1e5

Fix comment author check

5219469

Refactor comment body check

21a26e8

Provision service with active task to get correct IP

2e69ca0

/testenv up

a7db63f

superset-github-bot bot added the preset-io label Feb 17, 2021

pull-request-size bot added the size/L label Feb 17, 2021

robdiciuccio requested review from craig-rueda, dpgaspar, ktmud and nytai February 17, 2021 21:48

ktmud reviewed Feb 18, 2021

View reviewed changes

Add @mentions to PR comments, env var cleanup

6e5e572

killmen136 approved these changes Feb 19, 2021

View reviewed changes

rusackas requested a review from suddjian February 23, 2021 00:59

craig-rueda approved these changes Feb 23, 2021

View reviewed changes

robdiciuccio merged commit 27f7d11 into apache:master Feb 24, 2021

robdiciuccio deleted the rd/ephemeral-env branch February 24, 2021 18:50

robdiciuccio mentioned this pull request Feb 24, 2021

build: Use redis:latest in docker-compose workflow #13323

Merged

6 tasks

ad-m mentioned this pull request Jan 21, 2022

Testing strategy of Helm Chart #18127

Closed

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.2.0 labels Mar 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: Ephemeral environments for PRs via slash command #13189

build: Ephemeral environments for PRs via slash command #13189

robdiciuccio commented Feb 17, 2021 •

edited

Loading

ktmud commented Feb 17, 2021 •

edited

Loading

junlincc commented Feb 17, 2021

ktmud left a comment

ktmud Feb 17, 2021

robdiciuccio Feb 18, 2021

ktmud Feb 18, 2021

robdiciuccio commented Feb 18, 2021

ktmud commented Feb 18, 2021

robdiciuccio commented Feb 19, 2021

robdiciuccio commented Feb 19, 2021

craig-rueda left a comment

robdiciuccio commented Feb 24, 2021

junlincc commented Feb 24, 2021

robdiciuccio commented Feb 24, 2021

build: Ephemeral environments for PRs via slash command #13189

build: Ephemeral environments for PRs via slash command #13189

Conversation

robdiciuccio commented Feb 17, 2021 • edited Loading

SUMMARY

Running

Security

TEST PLAN

TODO (in future PRs)

ADDITIONAL INFORMATION

ktmud commented Feb 17, 2021 • edited Loading

junlincc commented Feb 17, 2021

ktmud left a comment

Choose a reason for hiding this comment

ktmud Feb 17, 2021

Choose a reason for hiding this comment

robdiciuccio Feb 18, 2021

Choose a reason for hiding this comment

ktmud Feb 18, 2021

Choose a reason for hiding this comment

robdiciuccio commented Feb 18, 2021

ktmud commented Feb 18, 2021

robdiciuccio commented Feb 19, 2021

robdiciuccio commented Feb 19, 2021

craig-rueda left a comment

Choose a reason for hiding this comment

robdiciuccio commented Feb 24, 2021

junlincc commented Feb 24, 2021

robdiciuccio commented Feb 24, 2021

robdiciuccio commented Feb 17, 2021 •

edited

Loading

ktmud commented Feb 17, 2021 •

edited

Loading