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

Add the ability to disable Atlantis locking a repo #1340

Merged
merged 3 commits into from
Jan 14, 2021
Merged

Add the ability to disable Atlantis locking a repo #1340

merged 3 commits into from
Jan 14, 2021

Conversation

gezb
Copy link
Contributor

@gezb gezb commented Jan 7, 2021

This change introduces a no-op Locker implementation that when enabled effectively disables the locking of projects and workspace

This is another possible solution to #1212, my initial change just automatically unlock after a plan had run (#1258)

@codecov
Copy link

codecov bot commented Jan 7, 2021

Codecov Report

Merging #1340 (6cf783f) into master (6b36adb) will increase coverage by 0.02%.
The diff coverage is 86.95%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1340      +/-   ##
==========================================
+ Coverage   70.01%   70.03%   +0.02%     
==========================================
  Files          74       74              
  Lines        5556     5570      +14     
==========================================
+ Hits         3890     3901      +11     
- Misses       1307     1308       +1     
- Partials      359      361       +2     
Impacted Files Coverage Δ
cmd/server.go 79.09% <ø> (ø)
server/events/comment_parser.go 93.98% <ø> (ø)
server/user_config.go 100.00% <ø> (ø)
server/server.go 61.97% <50.00%> (-0.28%) ⬇️
server/events/locking/locking.go 100.00% <100.00%> (ø)
server/events/markdown_renderer.go 92.30% <100.00%> (+0.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6b36adb...6cf783f. Read the comment docs.

server/server.go Outdated Show resolved Hide resolved
Co-authored-by: kbaldyga <kbaldyga@gmail.com>
@gezb
Copy link
Contributor Author

gezb commented Jan 9, 2021

I'm not sure if I should also not render To delete all plans and locks for the PR, comment: atlantis unlock when commenting back on the pr if this option is enabled?

@acastle
Copy link
Contributor

acastle commented Jan 11, 2021

Definitely a bigger fan of this approach than #1258. Disabling the unlock message as you have done seems appropriate to me as this is what we have done for other flags in the past.

Copy link
Contributor

@acastle acastle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

@kevincox
Copy link

I have one question about this. Forgive me for not understanding Atlantis internals.

This seems to disable the automatic lock taken by Atlantis. I think this is very helpful. However does the current approach allow manual locking? For example when doing manual work or otherwise applying outside of Atlantis it often is useful to take a "manual" lock. Would this be incompatible with that?

@gezb
Copy link
Contributor Author

gezb commented Jan 11, 2021

Hi @kevincox yes this option would be incompatible with taking a manual lock as the call to take any repo/project lock does not do anything,

When I revisited this I looked at the easiest way to disable locking without having to make too may changes to the internals of Atlantis because a lot of the comments on the issue around this and my other attempt were along the lines of why bother locking if you are just unlocking once the plan had been run and pointing out the fact that Terraform does it own locking

@kevincox
Copy link

I think this is still an improvement as currently our team just ignores locks anyways because most of them are pointless. However it would be nice at some point in the future to have support for manual locking. Since the noise is down this would provide a clear signal that someone is working on something.

@acastle acastle merged commit 1137a82 into runatlantis:master Jan 14, 2021
@gezb gezb deleted the feature/option_to_disable_locking_of_repos branch January 15, 2021 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants