feat: plan queue functionality #1923

ghaiszaher · 2021-11-29T16:03:30Z

Relates to: #1722

Hey @nishkrishnan, based on above discussion, we are submitting this PR that provides plan queue functionality.

The implementation of the queue functionality is complete and the code is almost production-ready. We are seeking your assistance and would appreciate your feedback if this is the right direction and if this functionality could be considered.

We we have done so far:

Automatically entering the queue upon a plan request if another PR has the lock.
Granting the lock to the next PR in queue when the previous PR is unlocked (either via explicit atlantis unlock command, from Atlantis UI or by merging/closing the PR).
Notifying the author that the PR is ready to be planned for the unlocked environments/workspaces.
- We avoided automatic planning because: 1) A PR can have more than one lock and not all of them might be released yet. 2) It was cumbersome to store the initial command in case extra arguments were passed.
Showing the queue in the main UI and in the locks view.

What is missing:

Feature flag to enable/disable the queue.
Handling plan errors in respect to the queue: currently the lock is released on plan error, which means we should either:
- Grant the lock to the next person in the queue
- Or change that behavior and not release the lock on plan errors
More commands to handle the queue: e.g. jump in front of the queue, or get out of the queue.
More test coverage.

Contributors

@monikma @amir-elgayed @ghaiszaher

* Dequeue next on Atlantis unlock * Dequeue next on PR cleanup (e.g. after closing) * Quick fix * Fix * Reworked the TODOs, docu and a bit of refactoring

* Display queue details in index and lock-details pages

* feat: improving apply requirements for mergeable on Gitlab (#1675) * Improving apply requirements for mergeable on Gitlab * Fixing end of line * Renaming variables * Renaming variables * Fixing variable assignment * Fixing variable assignment * fix ignore of --parallel-pool-size flag (#1705) * Fixing apply permission when MR status is draft (#1736) * Create bug_report.md (#1725) * Refactor Apply Requirements into a Handler. (#1729) * deps: tf 1.0.4 (#1748) * build(deps): bump tar from 4.4.4 to 4.4.15 (#1733) Bumps [tar](https://github.com/npm/node-tar) from 4.4.4 to 4.4.15. - [Release notes](https://github.com/npm/node-tar/releases) - [Changelog](https://github.com/npm/node-tar/blob/main/CHANGELOG.md) - [Commits](isaacs/node-tar@v4.4.4...v4.4.15) --- updated-dependencies: - dependency-name: tar dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump github.com/hashicorp/go-getter from 1.5.5 to 1.5.6 (#1728) Bumps [github.com/hashicorp/go-getter](https://github.com/hashicorp/go-getter) from 1.5.5 to 1.5.6. - [Release notes](https://github.com/hashicorp/go-getter/releases) - [Changelog](https://github.com/hashicorp/go-getter/blob/main/.goreleaser.yml) - [Commits](hashicorp/go-getter@v1.5.5...v1.5.6) --- updated-dependencies: - dependency-name: github.com/hashicorp/go-getter dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Lucas C <[email protected]> Co-authored-by: Pavel Golubev <[email protected]> Co-authored-by: Luke Kysow <[email protected]> Co-authored-by: Aayush Gupta <[email protected]> Co-authored-by: rui <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

chore: fetch upstream (2)

Revert the changes related to dequeue lock status message in original PR

… generate <path/to/interface.go>)

…tdb::ListQueues

…erge master)

* Move inline css to file

…implement-getQueueByLock

Merge from upstream

# Conflicts: # server/events/unlock_command_runner.go

GH-21 Change comment on unlock PR - notify lock creator

#10 Refactoring

merge upstream

fetch upstream

# Conflicts: # server/core/locking/mocks/mock_backend.go # server/events/unlock_command_runner.go

Merge upstream

GabrielBB · 2022-05-16T19:13:30Z

@ghaiszaher @monikma Great job

amir-elgayed · 2022-05-17T14:54:08Z

This also relates to #305 but is technically different.

@majormoses As far as I understood, it is quiet different: This is about providing a queueing mechanism for plans cross different PRs that affect the same workspace (they compete for the same lock), if a lock is acquired by PR#1, plans from other PRs will enter a queue and will be notified when the lock is released by PR#1, whereas #305 is about plans on the same PR that is already holding the lock.

merge from upstream

jamengual · 2022-08-18T16:54:33Z

@ghaiszaher is there still interest in getting this merged?

ghaiszaher · 2022-08-19T15:06:32Z

@ghaiszaher is there still interest in getting this merged?

@jamengual yes we are still interested, we'd appreciate any feedback on the current PR.

lilincmu

Except for few nits, this looks great to me! Thank you all for your contribution! 🎉

One more nit from me is, it might be more readable if we define []models.ProjectLock to be a new type, such as ProjectLockQueue. When reading the code, sometimes I had to figure out whether []models.ProjectLock refers to a queue, or simply a list of locks, for example, that are just being unlocked by a PR.

server/events/project_locker.go

server/core/db/boltdb.go

server/core/locking/locking.go

lilincmu · 2022-08-25T22:05:26Z

server/core/locking/locking.go

+	return c.backend.GetQueueByLock(project, workspace)
+}
+
+// TODO monikma extend the tests


Has this done? I'm guess you were going to verify DequeueStatus?

No, and yes..

lilincmu · 2022-08-25T22:06:40Z

server/events/delete_lock_command.go

-	lock, err := l.Locker.Unlock(id)
+func (l *DefaultDeleteLockCommand) DeleteLock(id string) (*models.ProjectLock, *models.ProjectLock, error) {
+	// TODO monikma extend the tests
+	// TODO monikma #9 When the lock(s) has(ve) been explicitly removed, also dequeue the next PR(s)


Are we going to extend the tests?

lilincmu · 2022-08-25T22:07:44Z

server/events/pull_closed_executor.go

@@ -99,14 +99,15 @@ func (p *PullClosedExecutor) CleanUpPull(repo models.Repo, pull models.PullReque
 		}
 	}

+	// TODO monikma extend the tests


Has this done?

server/events/project_locker.go

lilincmu · 2022-08-25T22:10:44Z

server/events/project_locker.go

@@ -59,6 +59,7 @@ type TryLockResponse struct {

 // TryLock implements ProjectLocker.TryLock.
 func (p *DefaultProjectLocker) TryLock(log logging.SimpleLogging, pull models.PullRequest, user models.User, workspace string, project models.Project) (*TryLockResponse, error) {
+	// TODO monikma extend the tests


Has this done?

jamengual · 2022-09-21T04:33:18Z

@ghaiszaher do you have any time to address the comments?

server/core/db/boltdb.go

server/core/locking/locking.go

monikma · 2022-09-21T21:57:17Z

@ghaiszaher do you have any time to address the comments?

@jamengual I tried to answer the comments, since Ghais is currently on vacation. First of all, @lilincmu thanks a lot for the review! Hopefully we can shorten the feedback cycle now, and make each next interaction exponentially quicker ;)

One more nit from me is, it might be more readable if we define []models.ProjectLock to be a new type, such as ProjectLockQueue. When reading the code, sometimes I had to figure out whether []models.ProjectLock refers to a queue, or simply a list of locks, for example, that are just being unlocked by a PR.

Makes a lot of sense to me.

About your questions on the tests - yes we have not implemented all of them, as we first wanted to check with you if this is some functionality that sounds like a good idea, and if how we go about implementing it would make sense to you (as mentioned in the PR description). So my understanding is "yes it does" ? :)

We could continue by applying the suggestions and extending tests. Would that make sense to you? What do you expect from us to call our PR "production ready", and get closer to merging it?

One topic we circled around but never properly got to, is feature flag to enable/disable the feature. If you had any suggestions there, it would be great (meaning how feature flags are usually approached in this codebase, we would not want to break some convention).

runatlantis#1923 (review)

runatlantis#1923 (comment)

Review comments from runatlantis#1923 (review) * no need for default https://github.com/runatlantis/atlantis/pull/1923/files/dc2506f0d810f9ebc5ea2e349e0a924108f991a9#r954286134 * rename to QueueDepth https://github.com/runatlantis/atlantis/pull/1923/files/dc2506f0d810f9ebc5ea2e349e0a924108f991a9#r955458720 * ProjectLockQueue is more meaningful name runatlantis#1923 (review) * Update doc runatlantis#1923 (comment) * remove todo https://github.com/runatlantis/atlantis/pull/1923/files/dc2506f0d810f9ebc5ea2e349e0a924108f991a9#r954323702 * update doc https://github.com/runatlantis/atlantis/pull/1923/files/dc2506f0d810f9ebc5ea2e349e0a924108f991a9#r954323951

ghaiszaher · 2023-01-29T15:01:37Z

@lilincmu @jamengual Since this PR was closed because the repo was deleted from the organization, I raised a new PR: #2782 which is in a better state now

cc @monikma @amir-elgayed

ghaiszaher · 2023-01-29T15:02:01Z

Except for few nits, this looks great to me! Thank you all for your contribution! 🎉

One more nit from me is, it might be more readable if we define []models.ProjectLock to be a new type, such as ProjectLockQueue. When reading the code, sometimes I had to figure out whether []models.ProjectLock refers to a queue, or simply a list of locks, for example, that are just being unlocked by a PR.

Thank you @lilincmu, renamed as part of this commit: 06a83e3

runatlantis#1923 (comment)

* fix doc runatlantis#1923 (comment) * handle json marshal errors runatlantis#1923 (comment)

ghaiszaher and others added 29 commits August 2, 2021 16:39

Enter the queue when lock not available (#12)

0334944

Dequeue next when lock available (#13)

302f774

* Dequeue next on Atlantis unlock * Dequeue next on PR cleanup (e.g. after closing) * Quick fix * Fix * Reworked the TODOs, docu and a bit of refactoring

Amir changes (#15)

eae4d9f

* Display queue details in index and lock-details pages

pass dequeueLock by reference (#17)

34a2c21

Merge pull request #20 from runatlantis/master

89bbfa0

chore: fetch upstream (2)

revert the changes related to dequeue lock status message in original PR

d288eae

comment on unlock pull requests and mention lock creator

06f3bac

add todos

8fa66a4

Merge pull request #23 from ScaCap/gh-21/remove-dequeued-comment

5599b2c

Revert the changes related to dequeue lock status message in original PR

Remove SyncPull method

e8bc849

Introduced GetQueueByLock method

3469d40

We changed some interfaces contract, regenerating mocks for those (go…

ccf16a7

… generate <path/to/interface.go>)

Fix tests in boltdb_test.go, added one experimental test case for bol…

ee487e9

…tdb::ListQueues

Remove SyncPull method from proxy.go (not needed anymore as we wont m…

7e17b27

…erge master)

Remove ListQueues method

8b0eeb5

#16: Remove inline css (#26)

90aadae

* Move inline css to file

Merge branch 'master' of https://github.com/ScaCap/atlantis into #10_…

e07f046

…implement-getQueueByLock

Merge pull request #22 from runatlantis/master

8cb6f32

Merge from upstream

commentOnDequeuedPullRequests when PR is closed

0d24871

modify todo

33420a0

fixes & comment on unlock from UI

4757f82

Merge branch 'master' into gh-21/change-comment-on-unlock-pr

8af33b9

# Conflicts: # server/events/unlock_command_runner.go

fix merge conflict

f6270a3

Merge pull request #24 from ScaCap/gh-21/change-comment-on-unlock-pr

ad81b6b

GH-21 Change comment on unlock PR - notify lock creator

Merge branch 'master' into #10_implement-getQueueByLock

833bf4a

Merge pull request #25 from ScaCap/#10_implement-getQueueByLock

761fe20

#10 Refactoring

Merge pull request #27 from runatlantis/master

1108041

merge upstream

Merge pull request #28 from runatlantis/master

f7a9655

fetch upstream

ghaiszaher changed the title ~~[DRAFT] Plan queue functionality~~ feat: plan queue functionality Nov 29, 2021

jamengual added the waiting-on-review Waiting for a review from a maintainer label May 13, 2022

monikma mentioned this pull request May 16, 2022

1722 plan queue functionality #1727

Closed

ghaiszaher added 3 commits May 16, 2022 14:39

Merge remote-tracking branch 'upstream/master' into merge-upstream

8b7699b

# Conflicts: # server/core/locking/mocks/mock_backend.go # server/events/unlock_command_runner.go

fix conflicts

f5ae318

Merge pull request #40 from ScaCap/merge-upstream

b2c1c91

Merge upstream

ghaiszaher marked this pull request as ready for review May 16, 2022 15:19

ghaiszaher requested a review from a team as a code owner May 16, 2022 15:19

Merge pull request #53 from runatlantis/master

dc2506f

merge from upstream

lilincmu reviewed Aug 25, 2022

View reviewed changes

monikma reviewed Sep 21, 2022

View reviewed changes

server/core/db/boltdb.go Show resolved Hide resolved

monikma reviewed Sep 21, 2022

View reviewed changes

server/core/db/boltdb.go Show resolved Hide resolved

monikma reviewed Sep 21, 2022

View reviewed changes

server/core/locking/locking.go Show resolved Hide resolved

ScaCap closed this by deleting the head repository Oct 5, 2022

nitrocode mentioned this pull request Dec 12, 2022

Add company Logos and success histories to the Atlantis page #2626

Open

This was referenced Dec 12, 2022

feat: plan queue functionality #2781

Closed

feat: plan queue functionality #2782

Open

ghaiszaher added a commit to ghaiszaher/atlantis that referenced this pull request Jan 29, 2023

ProjectLockQueue is more meaningful name

9dc170a

runatlantis#1923 (review)

ghaiszaher added a commit to ghaiszaher/atlantis that referenced this pull request Jan 29, 2023

Update doc

8ee0a78

runatlantis#1923 (comment)

ghaiszaher added a commit to ghaiszaher/atlantis that referenced this pull request Jan 29, 2023

fix doc

9ae8e5f

runatlantis#1923 (comment)

ghaiszaher mentioned this pull request Jan 29, 2023

review comments - part 2 ghaiszaher/atlantis#43

Merged

ghaiszaher added a commit to ghaiszaher/atlantis that referenced this pull request Jan 29, 2023

review comments - part 2 (#43)

7ed3fd3

* fix doc runatlantis#1923 (comment) * handle json marshal errors runatlantis#1923 (comment)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: plan queue functionality #1923

feat: plan queue functionality #1923

ghaiszaher commented Nov 29, 2021 •

edited

Loading

GabrielBB commented May 16, 2022

amir-elgayed commented May 17, 2022 •

edited

Loading

jamengual commented Aug 18, 2022

ghaiszaher commented Aug 19, 2022

lilincmu left a comment

lilincmu Aug 25, 2022

monikma Sep 21, 2022

lilincmu Aug 25, 2022

lilincmu Aug 25, 2022

lilincmu Aug 25, 2022

jamengual commented Sep 21, 2022

monikma commented Sep 21, 2022 •

edited

Loading

ghaiszaher commented Jan 29, 2023

ghaiszaher commented Jan 29, 2023

feat: plan queue functionality #1923

feat: plan queue functionality #1923

Conversation

ghaiszaher commented Nov 29, 2021 • edited Loading

We we have done so far:

What is missing:

Contributors

GabrielBB commented May 16, 2022

amir-elgayed commented May 17, 2022 • edited Loading

jamengual commented Aug 18, 2022

ghaiszaher commented Aug 19, 2022

lilincmu left a comment

Choose a reason for hiding this comment

lilincmu Aug 25, 2022

Choose a reason for hiding this comment

monikma Sep 21, 2022

Choose a reason for hiding this comment

lilincmu Aug 25, 2022

Choose a reason for hiding this comment

lilincmu Aug 25, 2022

Choose a reason for hiding this comment

lilincmu Aug 25, 2022

Choose a reason for hiding this comment

jamengual commented Sep 21, 2022

monikma commented Sep 21, 2022 • edited Loading

ghaiszaher commented Jan 29, 2023

ghaiszaher commented Jan 29, 2023

ghaiszaher commented Nov 29, 2021 •

edited

Loading

amir-elgayed commented May 17, 2022 •

edited

Loading

monikma commented Sep 21, 2022 •

edited

Loading