Atlantis: Make locking behaviour configurable

[lkysow]

Currently Atlantis locks a specific repo/directory/workspace combination. So if you run plan in subdir/ for workspace default in runatlantis/atlantis, another pull request will be able to run plan in subdir/ for workspace new in runatlantis/atlantis. This needs to be configurable.

From @lorenzoaiello they would like to be able to lock all directories and workspaces when there's a plan. This is because they have small repos and so if there's a change going on in any of the directories they would like the whole repo locked.

From @grobie, his team would like to completely disable locking because they use a mono-repo and so it would be too disruptive to lock things. He'd like plan to always be available.

[mechastorm]

his team would like to completely disable locking because they use a mono-repo and so it would be too disruptive to lock things. He'd like plan to always be available.

This will also be of great use to my team as we have some major teams that have large monorepos.

[lkysow]

@mechastorm do your monorepos have multiple directories though? Because each lock is per directory.
Even with this, grobie wanted the ability to disable it so his team can run plans even when there's a PR in the same directory because there's such a high volume they are okay with a "first to apply everyone else needs to rebase" workflow so they don't block each other.

[jvperrin]

@lkysow I'd also be interested in being able to disable locking so that there can be multiple PRs in flight for a monorepo. The lock contention can be pretty rough as is, especially if pull requests are left open for a few days. Sometimes asking a PR owner to close and then reopen after you've made a plan on your review to swap over the locks works, but that still feels more like a workaround than a real long-term solution, especially as the repo grows in activity.

Would having no locking still be feasible as long as everyone has to rebase or merge in master to update their branch on any applies, or have times changed since your last comment? I know it's been a while!

[lkysow]

Hi Jason, locking is required to avoid issues where a PR hasn't been merged but has been applied, see: https://www.runatlantis.io/docs/locking.html#why

[jvperrin]

That definitely makes sense. I've read that document but hadn't really thought about it in the context of applies that fail, so that's helpful.

However, I think most of our locking problems come from reviews that haven't been merged but also haven't been applied yet. Because planning takes that lock, it then means other reviews are unable to plan. Would it be possible to have the locking portion be just around the apply (and hang onto the lock if the apply fails, but release it as usual if it succeeds and the PR is merged)? I think we'd still run into some lock contention then, but at a much lower rate than it currently is.

Thanks for such a quick response! I've also really been enjoying using atlantis versus our previous much more manual workflow, so thanks for making something that's great to use!

[lkysow]

Would it be possible to have the locking portion be just around the apply

Right now we assume if there was a successful plan then applies are allowed. In this case we could no longer assume that because the plan might be out of date. One thing we could maybe do it show the plan in a comment but not actually save it (in the case of running when the project is locked) so it could never be applied. This is a pretty big change though.

[jvperrin]

Ah I see, that makes sense. Maybe having two different types of plans (locking and not) would work as you mention, but I do agree that's a pretty big change to atlantis overall, so not one to be made lightly.

So for some background, we currently have pull requests with GitHub set up with a protected master branch so that it requires the branch to be up to date with master before it's considered mergeable (as mentioned as a possiblity in https://www.runatlantis.io/docs/apply-requirements.html#supported-requirements). I think that would also ensure that when an apply is made that the most recent plan that it is using is already up to date (since for an apply to actually happen on a mergeable review the branch needs to be updated, and for when that branch update is done a new plan will be made). Does that seem reasonable as a workflow where just having locking around the apply would be reasonable, or would there be some better way to reduce lock contention? Splitting up projects is one way to do it since the lock is by project, but I'm trying to see if there's any other paths we could take to make things better before having to do that.

Thanks!

[lkysow]

I think that's a reasonable workflow. You could test out disabling locking by running a fork and changing this function to:

func (p *DefaultProjectLocker) TryLock(log *logging.SimpleLogger, pull models.PullRequest, user models.User, workspace string, project models.Project) (*TryLockResponse, error) {
    return &TryLockResponse{
        LockAcquired: true,
        UnlockFn:     func() error { return nil },
        LockKey:      fmt.Sprintf("%s/%s/%s", pull.BaseRepo.FullName, project.Path, workspace),
    }, nil

[jvperrin]

Nice, I'll give that try and see how it goes. Thanks for all your help with this!

[jlsjonas]

Maybe a related scenario: we have several environments loosely tied to certain branches, via a CI/CD process that runs terraform apply as part of the build chain. However we have an environment dedicated to our master, where we wanted to plan to (using atlantis).
This is working fine... as long as there's only 1 open PR changing terraform config at a time.

As we don't actually apply from atlantis, it would also be great for us if we could skip the locking.