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

fix: Discarding a Plan Causes the Whole Working Directory to be Deleted #3553

Merged
merged 8 commits into from
Jul 7, 2023

Conversation

X-Guardian
Copy link
Contributor

@X-Guardian X-Guardian commented Jun 25, 2023

what

  • In the Locks Controller DeleteLock function, remove the WorkingDirLocker.TryLock and WorkingDir.DeleteForWorkspace function calls as these are not needed as the function has already called DeleteLockCommand.DeleteLock.
  • In the DeleteLockCommand.DeleteLock function, replace the call to deleteWorkingDir with a call to the new function WorkingDir.DeletePlan. Note: projectName has to be hard coded to an empty string as the Locks Controller currently has no knowledge of Atlantis project names.
  • Add the WorkingDir.DeletePlan function to the events package with code to build the required plan file path and the delete the file. A logger has also been added to the FileWorkspace type.

why

tests

Tested locally

@X-Guardian X-Guardian requested a review from a team as a code owner June 25, 2023 13:37
@github-actions github-actions bot added the go Pull requests that update Go code label Jun 25, 2023
@jamengual
Copy link
Contributor

@GenPage

@X-Guardian
Copy link
Contributor Author

Maintainers, this PR is ready for review.

@jamengual
Copy link
Contributor

@GenPage relates to ADRs

Copy link
Member

@GenPage GenPage left a comment

Choose a reason for hiding this comment

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

This is a good change but I fear incomplete in the grand scheme of the ADR-002 (#3345 . Right now the workingDir and locks controller don't have a complete understanding of project names which you have found.

The focus with the ADR is to clean up previous code that attempted to add paths to the workingDir/workingDirLocker. It also tries to help contributors understand the call stack to improve unnecessary cloning when operating on repos.

I'm not trying to block the PR. My main concern is around completely removing the workingDir lock for releasing locks/deleting plans. Happy to discuss further.

@X-Guardian
Copy link
Contributor Author

X-Guardian commented Jul 7, 2023

Hi @GenPage, thanks for the feedback. I understand your comments, and agree that further work is required in the future with regard to the lock handling. I don't believe the changes in this PR block any future changes or make them harder though. Users can still release all locks with the atlantis unlock command and all these changes do is fix the really annoying issue of when you currently release one lock, all plans for the PR are deleted.

@GenPage GenPage enabled auto-merge (squash) July 7, 2023 15:11
Copy link
Member

@GenPage GenPage left a comment

Choose a reason for hiding this comment

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

Looks good to me

@GenPage GenPage merged commit 91593b6 into runatlantis:main Jul 7, 2023
@X-Guardian X-Guardian deleted the delete-lock-fix branch July 7, 2023 16:17
ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024
ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go Pull requests that update Go code
Projects
None yet
3 participants