Skip to content

Commit

Permalink
Make entire docker-publish workflow atomic (#178)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #178

There is actually one more concurrency issue. Consider the following scenario:

1) Commit A runs `build_prerelease_image` and uploads the prerelease image to `test_registry:test_image`
2) Commit A gets preempted by Github, and now Commit B does the same thing.
3) `test_registry:test_image` now contains Commit B's image, and Commit A's image is lost
4) when both Commit A's and Commit B's `private_lift_e2e_test` runs, it uses Commit B's image

Commit A will never get tested

Unfortunately, I think this means we have to make the entire workflow atomic. We only have one self hosted runner so it doesn't really change much for us.

Interestingly, fbpcs has always done it this way, so I guess they had more foresight than me :)

The proper solution would be to see if we can specify the image tag when running on the PCE, but at the moment that seems impossible since the image location is configured in the AWS console

Reviewed By: wuman

Differential Revision: D35405964

fbshipit-source-id: 633179e3a0e2da3ab157509d7010823b7da22cb6
  • Loading branch information
adshastri authored and facebook-github-bot committed Apr 7, 2022
1 parent 3ec2f58 commit 66f2d1b
Showing 1 changed file with 3 additions and 50 deletions.
53 changes: 3 additions & 50 deletions .github/workflows/docker-publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,16 @@ env:
COORDINATOR_IMAGE_DEV: ghcr.io/facebookresearch/fbpcs/coordinator:dev

jobs:
build_prerelease_images:
test_and_push:
runs-on: [self-hosted, e2e_test_runner]
permissions:
contents: write
packages: write

steps:
- uses: actions/checkout@v2
with:
fetch-depth: 0

- name: Build fbpcf docker image
run: |
Expand Down Expand Up @@ -73,19 +75,6 @@ jobs:
run: |
docker push --all-tags ${{ env.TEST_REGISTRY_IMAGE_NAME }}
private_attribution_e2e_test:
runs-on: [self-hosted, e2e_test_runner]
needs: build_prerelease_images
steps:
- uses: actions/checkout@v2

- name: Log into registry ${{ env.REGISTRY }}
uses: docker/login-action@v1
with:
registry: ${{ env.REGISTRY }}
username: ${{ github.actor }}
password: ${{ secrets.GITHUB_TOKEN }}

- name: Pull coordinator image
run: |
docker pull ${{ env.COORDINATOR_IMAGE_DEV }}
Expand Down Expand Up @@ -198,23 +187,6 @@ jobs:
docker stop ${{ env.PA_CONTAINER_NAME }}
docker rm ${{ env.PA_CONTAINER_NAME }}
private_lift_e2e_test:
runs-on: [self-hosted, e2e_test_runner]
needs: build_prerelease_images
steps:
- uses: actions/checkout@v2

- name: Log into registry ${{ env.REGISTRY }}
uses: docker/login-action@v1
with:
registry: ${{ env.REGISTRY }}
username: ${{ github.actor }}
password: ${{ secrets.GITHUB_TOKEN }}

- name: Pull coordinator image
run: |
docker pull ${{ env.COORDINATOR_IMAGE_DEV }}
- name: Start container
run: |
./start_container.sh ${{ env.PL_CONTAINER_NAME }} ${{ env.COORDINATOR_IMAGE_DEV }}
Expand Down Expand Up @@ -334,25 +306,6 @@ jobs:
docker stop ${{ env.PL_CONTAINER_NAME }}
docker rm ${{ env.PL_CONTAINER_NAME }}
tag_and_push:
runs-on: [self-hosted, e2e_test_runner]
needs: [build_prerelease_images, private_attribution_e2e_test, private_lift_e2e_test]
permissions:
contents: write
packages: write

steps:
- uses: actions/checkout@v2
with:
fetch-depth: 0

- name: Log into registry ${{ env.REGISTRY }}
uses: docker/login-action@v1
with:
registry: ${{ env.REGISTRY }}
username: ${{ github.actor }}
password: ${{ secrets.GITHUB_TOKEN }}

- name: Create version string
id: create_version
uses: paulhatch/[email protected]
Expand Down

0 comments on commit 66f2d1b

Please sign in to comment.