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

Sticky locks + multiple environments = some unexpected behaviour #316

Open
mnaser opened this issue Oct 22, 2024 · 4 comments
Open

Sticky locks + multiple environments = some unexpected behaviour #316

mnaser opened this issue Oct 22, 2024 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@mnaser
Copy link

mnaser commented Oct 22, 2024

Describe the Issue

I have a monorepo with all of my different environments listed in there. Not every commit is deployed to every environment (for example, we might make a change to env-a only, and then only .deploy to env-a. No need to touch the other environments).

We use sticky_locks and it seems that all locks are released when the unlock-on-merge happens.. is there a way to try and figure out what deployments ran in the PR that was merged, and then only unlocking that environment?

Action Configuration

name: branch-deploy

on:
  issue_comment:
    types: [created]

permissions:
  checks: read
  contents: write
  deployments: write
  id-token: write
  pull-requests: write

jobs:
  deploy:
    runs-on: ubuntu-latest
    if: ${{ github.event.issue.pull_request }}

    steps:
      - uses: github/[email protected]
        id: branch-deploy
        with:
          reaction: rocket
          permissions: admin
          sticky_locks: true
          production_environments: |-
            xxx,
            yyy,
            zzz,
          environment_targets: |-
            xxx,
            yyy,
            zzz,

      - uses: actions/checkout@v4
        if: ${{ steps.branch-deploy.outputs.continue == 'true' }}
        with:
          ref: ${{ steps.branch-deploy.outputs.ref }}

      - name: fake noop deploy
        if: ${{ steps.branch-deploy.outputs.continue == 'true' && steps.branch-deploy.outputs.noop == 'true' }}
        run: echo "I am doing a fake noop deploy"

      - name: Deploy environment
        uses: ./.github/actions/deploy
name: unlock-on-merge

on:
  pull_request:
    types: [closed]

permissions:
  contents: write

jobs:
  unlock-on-merge:
    runs-on: ubuntu-latest
    if: github.event.pull_request.merged == true

    strategy:
      fail-fast: false

    steps:
      - uses: github/[email protected]
        id: unlock-on-merge
        with:
          unlock_on_merge_mode: "true"
          environment_targets: |-
            xxx,
            yyy,
            zzz,

Relevant Actions Log Output

🏃 running in 'unlock on merge' mode
🔍 lock file does not exist on branch: xxx-branch-deploy-lock
🔍 lock file does not exist on branch: -yyy-branch-deploy-lock
🔍 lock file does not exist on branch: -zzz-branch-deploy-lock

Extra Information

I also noticed and find it strange there is an extra - .. I wonder if there is an extra line/space that is being added there that is breaking the locking.

@mnaser mnaser added the bug Something isn't working label Oct 22, 2024
@GrantBirki
Copy link
Member

GrantBirki commented Oct 22, 2024

👋 @mnaser

I think I see what the issue is here. The production_environments and environment_targets inputs are strictly expecting a comma separated listed of environments. I think the |- operator in your yaml is translating them in a way that is adding the - prefix onto each.

Try with this format instead and see if it presents with the same issue:

- uses: github/[email protected]
  id: branch-deploy
  with:
    reaction: rocket
    permissions: admin
    sticky_locks: true
    production_environments: xxx,yyy,zzz # <-- simple comma separated list
    environment_targets: xxx,yyy,zzz     # <-- simple comma separated list

make sure to do the same adjustments in your unlock-on-merge workflow too 😉

@GrantBirki GrantBirki self-assigned this Oct 22, 2024
@mnaser
Copy link
Author

mnaser commented Oct 22, 2024

Hey @GrantBirki -- just did that (kinda annoying, since we have like 20+ envs), but I'll try and report back shortly tomorrow.

@GrantBirki
Copy link
Member

Wow 20+ envs is impressive! I think that is a record for this Action and I'm glad to see that its working well other than this leading - issue.

Perhaps we can investigate in the future if there is a nicer input option (something like what you have listed above) rather than one jumbo comma separated string.

@GrantBirki
Copy link
Member

@mnaser How did your testing go? Is this issue still needed or can I close it out?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants