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: allow projects in separate directories to run in parallel #2131

Merged

Conversation

KevinSnyderCodes
Copy link
Contributor

@KevinSnyderCodes KevinSnyderCodes commented Mar 10, 2022

In the release notes for v0.13.0: https://github.com/runatlantis/atlantis/blob/master/CHANGELOG.md#features-4

Running in parallel is only supported if you're using workspaces to separate your projects. Projects in separate directories can not be run in parallel currently.

This PR adds path as an argument to DefaultWorkingDirLocker.TryLock() and includes path in the workspaceKey used to check if a project is locked. This allows projects in separate directories to run in parallel.

To my knowledge, there is no functional reason against or unintended side effects from projects in separate directories running in parallel.

Additional changes:

  • Update all calls to DefaultWorkingDirLocker.TryLock()
  • Update WorkingDirLocker interface and all implementations
  • Create new unit test TestTryLockWithDifferentPaths that tests the behavior of locking two separate directories with the same workspace name

In the release notes for v0.13.0: https://github.com/runatlantis/atlantis/blob/master/CHANGELOG.md#features-4

> Running in parallel is only supported if you're using workspaces to separate your projects. Projects in separate directories can not be run in parallel currently.

This commit adds `path` as an argument to `DefaultWorkingDirLocker.TryLock()` and includes `path` in the `workspaceKey` used to check if a project is locked. This should allow projects in separate directories to run in parallel.

To my knowledge, there is no functional reason that projects in separate directories cannot run in parallel.

All calls to `DefaultWorkingDirLocker.TryLock()` have been updated. A new unit test `TestTryLockWithDifferentPaths` has been added to test the behavior of locking two separate directories with the same workspace name.
@KevinSnyderCodes KevinSnyderCodes requested a review from a team as a code owner March 10, 2022 00:08
@KevinSnyderCodes KevinSnyderCodes changed the title fix: add path to DefaultWorkingDirLocker.TryLock() fix: allow projects in separate directories to run in parallel Mar 10, 2022
@jamengual
Copy link
Contributor

Looks good to me, could you add maybe a doc with an example usage?

@pauloconnor
Copy link
Contributor

Is there an ETA for cutting a new release with this in it?

KevinSnyderCodes pushed a commit to KevinSnyderCodes/atlantis that referenced this pull request Apr 4, 2022
`WorkingDirLocker` and `WorkingDir` are closely related -- the former acquires a lock, which ensures that the latter can safely operate on a given file path.

In runatlantis#2131 we added `path` to the `WorkingDirLocker` lock key, but neglected to add the same to `WorkingDir`.

This commit adds `path` as an argument to certain `WorkingDir` methods, and includes `path` in the directory that we use to clone the repository for a given project.

Since `path` can include certain special characters such as `/`, we encode `path` as base32 when using it as part of a file path. This should ensure no special characters are used in the filesystem, and that the value can be decoded if desired (unlike hashes such as md5).

Additional changes:

- All calls to changed methods have been updated, including unit tests
- Mocks have been regenerated for `WorkingDir` and `WorkingDirLocker`
- `working_dir_test.go`
  - Commands that operate on filesystem paths have been updated to include the base32 encoded `path` value
  - When running `git init`, we include `-b master` as additional arguments, to ensure that `master` is our default branch (expected by the unit tests)
jamengual pushed a commit that referenced this pull request Apr 8, 2022
* fix: add path to WorkingDir methods

`WorkingDirLocker` and `WorkingDir` are closely related -- the former acquires a lock, which ensures that the latter can safely operate on a given file path.

In #2131 we added `path` to the `WorkingDirLocker` lock key, but neglected to add the same to `WorkingDir`.

This commit adds `path` as an argument to certain `WorkingDir` methods, and includes `path` in the directory that we use to clone the repository for a given project.

Since `path` can include certain special characters such as `/`, we encode `path` as base32 when using it as part of a file path. This should ensure no special characters are used in the filesystem, and that the value can be decoded if desired (unlike hashes such as md5).

Additional changes:

- All calls to changed methods have been updated, including unit tests
- Mocks have been regenerated for `WorkingDir` and `WorkingDirLocker`
- `working_dir_test.go`
  - Commands that operate on filesystem paths have been updated to include the base32 encoded `path` value
  - When running `git init`, we include `-b master` as additional arguments, to ensure that `master` is our default branch (expected by the unit tests)

* Try fixing E2E tests

* Fix DefaultPendingPlanFinder

In addition to iterating over `workspaceDirs`, also iterate over `pathDirs`, and use both `workspace` and `path` to build `repoDir`.

* Fix DefaultPendingPlanFinder unit tests

* Fix DefaultProjectCommandBuilder unit tests

Co-authored-by: Kevin Snyder <kevinsnyder@KevinSnydersMBP.lan>
Co-authored-by: Kevin Snyder <kevinsnyder@ip-192-168-1-188.ec2.internal>
@andyshinn
Copy link

andyshinn commented May 27, 2022

I think maybe this introduces a bug? All of my PRs now start with error:

The default workspace at path . is currently locked by another command that is running for this pull request.
Wait until the previous command is complete and try again.

Then similar during apply:

The Atlantis working dir is currently locked by another command that is running for this pull request.
Wait until the previous command is complete and try again.

But the other workspaces continue. There is no default workspace in these projects.

@snorlaX-sleeps snorlaX-sleeps mentioned this pull request Sep 22, 2022
7 tasks
krrrr38 pushed a commit to krrrr38/atlantis that referenced this pull request Dec 16, 2022
…lantis#2131)

* fix: add path to DefaultWorkingDirLocker.TryLock()

In the release notes for v0.13.0: https://github.com/runatlantis/atlantis/blob/master/CHANGELOG.md#features-4

> Running in parallel is only supported if you're using workspaces to separate your projects. Projects in separate directories can not be run in parallel currently.

This commit adds `path` as an argument to `DefaultWorkingDirLocker.TryLock()` and includes `path` in the `workspaceKey` used to check if a project is locked. This should allow projects in separate directories to run in parallel.

To my knowledge, there is no functional reason that projects in separate directories cannot run in parallel.

All calls to `DefaultWorkingDirLocker.TryLock()` have been updated. A new unit test `TestTryLockWithDifferentPaths` has been added to test the behavior of locking two separate directories with the same workspace name.

* Add documntation for parallel_plan and parallel_apply options

Co-authored-by: Kevin Snyder <kevinsnyder@ip-192-168-4-61.ec2.internal>
Co-authored-by: Kevin Snyder <kevinsnyder@ip-10-60-10-94.ec2.internal>
krrrr38 pushed a commit to krrrr38/atlantis that referenced this pull request Dec 16, 2022
* fix: add path to WorkingDir methods

`WorkingDirLocker` and `WorkingDir` are closely related -- the former acquires a lock, which ensures that the latter can safely operate on a given file path.

In runatlantis#2131 we added `path` to the `WorkingDirLocker` lock key, but neglected to add the same to `WorkingDir`.

This commit adds `path` as an argument to certain `WorkingDir` methods, and includes `path` in the directory that we use to clone the repository for a given project.

Since `path` can include certain special characters such as `/`, we encode `path` as base32 when using it as part of a file path. This should ensure no special characters are used in the filesystem, and that the value can be decoded if desired (unlike hashes such as md5).

Additional changes:

- All calls to changed methods have been updated, including unit tests
- Mocks have been regenerated for `WorkingDir` and `WorkingDirLocker`
- `working_dir_test.go`
  - Commands that operate on filesystem paths have been updated to include the base32 encoded `path` value
  - When running `git init`, we include `-b master` as additional arguments, to ensure that `master` is our default branch (expected by the unit tests)

* Try fixing E2E tests

* Fix DefaultPendingPlanFinder

In addition to iterating over `workspaceDirs`, also iterate over `pathDirs`, and use both `workspace` and `path` to build `repoDir`.

* Fix DefaultPendingPlanFinder unit tests

* Fix DefaultProjectCommandBuilder unit tests

Co-authored-by: Kevin Snyder <kevinsnyder@KevinSnydersMBP.lan>
Co-authored-by: Kevin Snyder <kevinsnyder@ip-192-168-1-188.ec2.internal>
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.

4 participants