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

Option to disable locking. #1212

Closed
kevincox opened this issue Oct 6, 2020 · 8 comments
Closed

Option to disable locking. #1212

kevincox opened this issue Oct 6, 2020 · 8 comments
Labels
feature New functionality/enhancement

Comments

@kevincox
Copy link

kevincox commented Oct 6, 2020

Atlantis locking can be overly restrictive and can delay work on some PR because another PR is open. Some teams would prefer to use atlantis without locks.

For example if I open a PR for a locked project it would be nice if I could still plan and see the result. This way I can make fixes, improvements and a code review without waiting for the other PR to be submitted.

Additionally if my PR is ready to submit before the prior PR it would be nice (to have an option) to submit. This would invalidate the plan on the other PR but this may be preferable to blocking this PR. To make this even more transparent atlantis could automatically re-plan the older PR so that review can continue.

With these options enabled it would be possible to have multiple changes to the same project in-flight in parallel.

@lkysow lkysow added the feature New functionality/enhancement label Oct 7, 2020
@gezb
Copy link
Contributor

gezb commented Oct 28, 2020

@lkysow I would like to look at working on this as it would work well with the disable apply feature

I see a few options here when a new flag enabling this feature is set either:

  • When a plan is run for a PR on a locked project we could automatically unlock it and continue to plan (invaliding the previous PR's plan requiring it to be re-run)
  • Automatically unlock a project once a plan is successful

The second option would work best for my use case where I want to disable the apply functionally as that is handled externally, but I can see how this would break the Atlantis workflow as a PR would never be in the right state(have a valid plan) to apply

So I think I've talked myself into the first option but wanted to run this past you before I start to look at it, Does this sound sensible to you?

@kevincox
Copy link
Author

  • When a plan is run for a PR on a locked project we could automatically unlock it and continue to plan (invaliding the previous PR's plan requiring it to be re-run)

I don't think there is any need to invalidate the old plan until either of the PRs apply. In that case we could invalidate the existing plans (will be necessary unless the apply was a no-op). However even if we don't terraform will report an error due to a stale plan when you try to apply it. So I see some pretty good options:

  1. Pessimistically invalidate all plans on any apply.
  2. Optimistically keep plans. However detect stale plans during apply and auto-replan. (Human re-approval will be required for re-apply).
  3. Auto-replan all other PRs on apply. This is really nice for users (other than extra notification emails) but may be a resource use concern. (Imagine I break up some work into 6 PRs. Now I will trigger 5+4+3+2+1=15 re-plans if I submit them all).

The only concern is that there are some cases where there is good reason to take a lock. For example if you are importing some resources or applying locally. In this case we could allow plans, but may want to show a warning about spurious diffs (trying to undo the work-in-progress). In this case it is probably also best to be very explicit about allowing apply, as it will undo the WIP.

This makes me think that we may still want to keep the lock, but to stop using it for plans. Imagine this:

Common Case

  • Open PR 1.
  • This plans, but does not lock.
  • Open PR 2.
  • This plans, but also doesn't lock.
  • Apply either PR.
  • The other will now need to be re-planned before applying.

Explicit lock

  • Open PR 1.
  • atlantis lock to take an explicit lock.
  • Open PR 2. (Maybe plans, but can't apply without explicitly dropping the lock from PR 1).
  • Apply either PR.
  • The other will need to be re-planned.

I think in this case we can preserve the old locks (better to be safe than sorry). It is just that new PRs wouldn't take locks.

There is another case that might be interesting. This is taking a lock just for the apply process. If you imagine that a lot of people are trying to apply at once it can cause a lot of replans. In this case it may make sense to hold the lock after an atlantis apply if a stale-plan error occurs. Then atlantis will do an auto-replan and you don't have to worry about someone else getting ahead of you in line again. (Later polish to this feature could include maintaining a "ready to go" queue and deferring the auto-replan until you are at the front of the queue).

To summarize:

  1. Planning no longer takes a lock.
  2. Trying to takes a lock until successful (or explicit unlock).
  3. There is an explicit lock option.

Possible config options:

  • PR locking enabled/disable.
  • Apply locking enabled/disabled.
  • Don't allow planning if the lock is currently held. (Avoid spurious diffs)

This seems like the best case for my team. Does that make sense to you?

@ginski47
Copy link

ginski47 commented Nov 3, 2020

has this been added as a server side config? disabling locking?

@gezb
Copy link
Contributor

gezb commented Nov 12, 2020

Sorry its taken so long for me to get back to this

I don't think disabling locking altogether is a good idea - It was obviously added for a reason, and thinking about it I think the locking is tied into the plan output files so you need a successful plan and lock before an apply can be done

I have created #1258 which works for my use case where we don't want to use Atlantis to do the Terraform apply and want to use it for previewing changes (we have a separate workflow that does terraform apply when a PR is merged to master)

@kevincox
Copy link
Author

I think the locking is tied into the plan output files so you need a successful plan and lock before an apply can be done

The plans themselves can detect if they are stale, there is no need for the extra lock from this perspective.

IIUC the argument for the lock is so that people aren't confused when their apply fails. However if the team is aware of that it isn't an issue.

@dimisjim
Copy link
Contributor

dimisjim commented Jan 6, 2021

I don't think disabling locking altogether is a good idea - It was obviously added for a reason,

@gezb Isn't the terraform lock enough, in situations where you don't mind your plan getting invalidated by another PR?

@nwsparks
Copy link

nwsparks commented Mar 9, 2022

the ability to specify this in a command, such as atlantis plan -lock=false would be beneficial as well. it's useful for PR's that are in draft and you'd like to see a plan without tying things up for other PR's which are ready to go.

@nitrocode
Copy link
Member

@nwsparks currently locking can be disabled using --disable-repo-locking

https://www.runatlantis.io/docs/server-configuration.html#disable-repo-locking

See the related issue #2237 for the same feature youre proposing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality/enhancement
Projects
None yet
Development

No branches or pull requests

7 participants