Skip to content
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

Maintenance: Refactor artifact name strategy for parallel uploads #3748

Closed
2 tasks done
heitorlessa opened this issue Feb 9, 2024 · 5 comments
Closed
2 tasks done
Labels
internal Maintenance changes

Comments

@heitorlessa
Copy link
Contributor

Why is this needed?

GitHub actions/upload-artifact v4 has a racing condition when uploading artifacts with the same artifact name in parallel.

For example, our Lambda Layer CDK artifacts are uploaded on a per region deploy in parallel using the a static output name.

This caused today's release to fail partially - re-running failed jobs served as a workaround.

image

Which area does this relate to?

No response

Solution

Since it's a race condition on upload, we need a dynamic artifact name e.g., one per region. Then merge these artifacts on download.

Updating to a dynamic name

      - name: Save Layer ARN artifact
        if: ${{ inputs.stage == 'PROD' }}
        uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3 # v4.3.1
        with:
          name: cdk-layer-stack-{{ matrix.region }}
          path: ./layer/cdk-layer-stack/* # NOTE: upload-artifact does not inherit working-directory setting.
          if-no-files-found: error
          retention-days: 1
          overwrite: true   # still needed to allow failed retries

Downloading all artifacts for all regions

      - name: Download CDK layer artifact
        uses: actions/download-artifact@eaceaf801fd36c7dee90939fad912460b18a1ffe # v4.1.2
        with:
          name: cdk-layer-stack
          path: cdk-layer-stack/
          merge-multiple: true

We'll need a testing environment to create a parallel job to test this out.

Acknowledgment

@heitorlessa heitorlessa added triage Pending triage from maintainers internal Maintenance changes labels Feb 9, 2024
@heitorlessa
Copy link
Contributor Author

linking a sub-action solution if we also need it in the future: actions/upload-artifact#505

@heitorlessa
Copy link
Contributor Author

Working on this tomorrow morning so we can make a patch release

@dreamorosi
Copy link
Contributor

We're also planning on making a release in the afternoon; let us know if you want to team up or just bounce ideas.

@heitorlessa
Copy link
Contributor Author

heitorlessa commented Feb 19, 2024 via email

Copy link
Contributor

⚠️COMMENT VISIBILITY WARNING⚠️

This issue is now closed. Please be mindful that future comments are hard for our team to see.

If you need more assistance, please either tag a team member or open a new issue that references this one.

If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal Maintenance changes
Projects
Status: Shipped
Development

No branches or pull requests

3 participants