Delete previous plans on autoplan or atlantis plan #1633

giuli007 · 2021-06-15T15:09:32Z

This PR aims to fix #1624 (and #1122)

When using non-default workspaces, plans are stored in pr-and-workspace-specific directories.
This is often the case when people want to use the parallel plan/apply feature of Atlantis to speedup how long it takes to plan PRs that change a lot of projects at the same time

If a PR is updated after being created it might happen that some of the original plans are no longer relevant with regards
to the latest changes.

The change in this PR ensures that previous plans are always deleted when a generic plan is triggered either by autoplan
or by a "atlantis plan" command.

NB Even after this change plans are not going to be cleaned up when specific projects are planned explicitly with "atlantis plan -p/-d/-w" because it is reasonable to allow someone to progressively build a set of plans for different projects, with different "atlantis plan -p/-d/-w" comments, to then apply them all together.

I've added a couple of phrases to the documentation to clarify this.

nishkrishnan

The idea makes sense to me. I'm glad the change in functionality is documented here as well as that's bound to get some questions.

Only thing I'd mention is that the same problem exists with locks I would imagine. Ie. if you amend the change, the lock will still be present. The code for deleting the lock should automatically clean up the data. This is more generic than just deleting the plan file so I'd suggest doing that to harden this implementation.

nishkrishnan · 2021-06-22T16:16:07Z

runatlantis.io/docs/using-atlantis.md

@@ -45,6 +45,10 @@ atlantis plan -w staging
 * `-w workspace` Switch to this [Terraform workspace](https://www.terraform.io/docs/state/workspaces.html) before planning. Defaults to `default`. If not using Terraform workspaces you can ignore this.
 * `--verbose` Append Atlantis log to comment.

+::: warning NOTE
+A `terraform plan` (without flags), like autoplans, discards all plans previously created with `atlantis plan` `-p`/`-d`/`-w`


Suggested change

A `terraform plan` (without flags), like autoplans, discards all plans previously created with `atlantis plan` `-p`/`-d`/`-w`

An `atlantis plan` (without flags), like autoplans, discards all plans previously created with `atlantis plan` `-p`/`-d`/`-w`

Suggested change

A `terraform plan` (without flags), like autoplans, discards all plans previously created with `atlantis plan` `-p`/`-d`/`-w`

A `terraform plan` (without flags), like autoplans, discards all plans previously created with `atlantis plan` `-p`/`-d`/`-w`

fixed the using-atlantis.md thanks

giuli007 · 2021-06-30T17:40:07Z

Only thing I'd mention is that the same problem exists with locks I would imagine. Ie. if you amend the change, the lock will still be present. The code for deleting the lock should automatically clean up the data. This is more generic than just deleting the plan file so I'd suggest doing that to harden this implementation.

Thanks for the suggestion @nishkrishnan, I think that makes sense. I'll have a look at deleting the locks as well.

giuli007 · 2021-07-14T17:40:32Z

@nishkrishnan I have added changes to delete the locks.

As you suggested I have also considered and tried using one of the already existing methods to delete the lock in a different PR #1704.
That method however ends up completely removing the working directories.
There are some situations where I think atlantis benefits from keeping the working directories around especially when using non-default workspaces (which means each project in a PR is cloned to a different directory).
In those cases an atlantis plan will cause atlantis to remove all the workspaces directories for that a PR and re-clone them unnecessarily(because the code the PR is for hasn't changed). This is less bad if only default workspaces are used as that means re-cloning it only once.
For those reasons I think the current PR is a better solution as it shouldn't cause the potential performance degradation mentioned.

I am still open to opinions and would be ok if we decide to merge #1704 instead, let me know what you think.

srlightbody · 2021-07-21T21:20:02Z

Was just reviewing open issues and saw this, I think it will also resolve issue #1122 that I opened a while back.

angeloskaltsikis · 2021-07-23T13:35:09Z

Hey, @giuli007 thanks for both of your PRs.
As I have worked on trying to solve the same issue in the past as well (PR, I would like to say that not deleting all the previous folders would work better for our use case (Using Atlantis + Terragrunt), due to utilizing the .terragrunt-cache folder for the new plans.
In the #1704 I believe the above happens every time.
If I understand correctly, by the approach of this PR we avoid this, correct?

P.S.
In case you need any extra help with the above I would be glad to jump in.

giuli007 · 2021-07-27T15:24:14Z

Hi @srlightbody @angeloskaltsikis thanks for the feedback.

As I have worked on trying to solve the same issue in the past as well (PR, I would like to say that not deleting all the previous folders would work better for our use case (Using Atlantis + Terragrunt), due to utilizing the .terragrunt-cache folder for the new plans.
In the #1704 I believe the above happens every time.
If I understand correctly, by the approach of this PR we avoid this, correct?
Yes the current approach in this PR will not remove directories but only locks and previously created plans while #1704 removes everything for a specific PR (including your terragrunt cache if it is stored along the tf code).
This is the reason why I also prefer this approach.

Thanks for the pointers to #1399 and #1122 which are practically the same issue and a PR very similar to this one.

I see that @nishkrishnan concern was that all these fixes change the implicit functionality that atlantis currently has to stack up plans triggered by different atlantis plan commands and then apply them all together.

I guess the advantage of this PR is that it only discards previous plans in case an explicit change has been pushed to the PR (which would trigger autoplan) or an explicit generic atlantis plan is requested.
Alternatively a user can still build up a sequence of specific plans (with atlantis plan -p) and then apply them all together with atlantis apply.

My hope is that the changes to the docs in this PR will help making users aware of this behavioural change (I can make them even more explicit if people think that would be useful).

github-actions · 2021-08-28T01:47:08Z

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

giuli007 · 2021-09-14T18:20:41Z

@nishkrishnan was there anything more you wanted me to address? Do the changes in this PR look like they could be merged?

giuli007 · 2021-10-06T10:55:35Z

This PR has been automatically flagged as stale and closed because of lack of responses.
I do still think this is a useful fix (we have been using it for a few months now) and am interested on seeing it merged upstream, and I think other people on this thread agree.
@nishkrishnan or anyone of the maintainers (@chenrui333 / @lkysow / @anubhavmishra) can we reopen this PR?
I am happy to take the time to make it mergeable again and possibly address further concerns if any?

github-actions · 2022-02-18T01:52:32Z

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

zraider7 · 2022-05-05T21:46:10Z

This is still an issue that we face. Is there a problem with the proposed solution @nishkrishnan ? Can we re-open this please?

jamengual · 2022-05-05T22:43:52Z

@giuli007 if you want to fix the conflicts we could discuss on emerging this and try it out on the prerelease

github-actions · 2022-07-30T02:15:41Z

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

okgolove · 2022-07-30T10:26:29Z

No stale please 🙈

github-actions · 2022-08-30T02:16:25Z

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

giuli007 · 2022-08-30T06:47:26Z

no stale

giuli007 · 2022-09-06T17:24:12Z

I finally got round to update this.
I've rebased on top of v0.19.8 and run some local tests and verified that this still does what I wanted it to do: discard existing plans on autoplan or on atlantis plan commands, but not on specific atlantis plan -p <path> commands.

Out of excess of caution I went to look at the issues that the #1704 PR (alternative to this one) was causing when it was merged into v0.19.5-pre.20220622 and running that use-case (as I understand it: When autoplanning the second time (pushing an update to an open PR)) works without causing issues.
I wouldn't expect this PR to cause similar issues because it explicitly only deletes plans and locks, not workdirs. But it would perhaps be good if @ascandella can check it again once this goes into a pre-release.

@jamengual Let me know if you need anything else or we can merge it into a pre-release already.

When using non-default workspaces, plans are stored in pr-and-workspace-specific directories. If a PR is subsequently updated it might happen that some of the plans are no longer relevant with regards to the latest changes. This change ensures that plans are always deleted when a generic plan is triggered either by autoplan or by a "atlantis plan" command. NB Plans are not cleaned up when specific projects are planned explicitly with "atlantis plan -p/-d/-w".

autoplan or generic plan command

giuli007 · 2022-09-15T17:27:13Z

I've rebased on top of master / v0.19.9-pre.20220912 to resolve conflicts with the changes from #2491

Any chance that this can be included in a v0.19.9 pre-release ?
cc @jamengual @chenrui333 @nishkrishnan

jamengual · 2022-09-15T23:15:51Z

server/events/plan_command_runner.go

@@ -64,6 +67,7 @@ type PlanCommandRunner struct {
 	autoMerger                 *AutoMerger
 	parallelPoolSize           int
 	pullStatusFetcher          PullStatusFetcher
+	locker                     locking.Locker


This name does not follow the naming conversion of the struct

@jamengual I've now fixed this

lilincmu

This looks good to me! Thanks for your contribution 🎉

jamengual · 2022-09-23T22:26:02Z

@giuli007 https://github.com/runatlantis/atlantis/releases/tag/v0.19.9-pre.20220923

giuli007 · 2022-09-29T11:21:44Z

Thanks @jamengual @lilincmu

* Delete previous plans on autoplan or atlantis plan When using non-default workspaces, plans are stored in pr-and-workspace-specific directories. If a PR is subsequently updated it might happen that some of the plans are no longer relevant with regards to the latest changes. This change ensures that plans are always deleted when a generic plan is triggered either by autoplan or by a "atlantis plan" command. NB Plans are not cleaned up when specific projects are planned explicitly with "atlantis plan -p/-d/-w". * Also delete locks along with plans when running autoplan or generic plan command * Fix Plan deletion tests after 0.19.8 rebase * Fix Plan deletion tests after v0.19.9-pre.20220912 rebase * Rename struct field to follow camelCase naming convention Co-authored-by: giuli007 <[email protected]>

giuli007 requested a review from a team as a code owner June 15, 2021 15:09

giuli007 force-pushed the delete_potentially_stale_previous_plans_which_fixes_1624 branch from 38cb1a9 to 6dfecab Compare June 15, 2021 19:29

nishkrishnan reviewed Jun 22, 2021

View reviewed changes

nishkrishnan added the waiting-on-response Waiting for a response from the user label Jun 22, 2021

giuli007 force-pushed the delete_potentially_stale_previous_plans_which_fixes_1624 branch 2 times, most recently from 1b0079f to 46f6821 Compare June 25, 2021 18:17

giuli007 force-pushed the delete_potentially_stale_previous_plans_which_fixes_1624 branch 3 times, most recently from 86a14f8 to 4e73bfe Compare July 14, 2021 16:59

giuli007 mentioned this pull request Jul 14, 2021

fix: Delete locks and workdirs with potentially stale previous plans which fixes 1624 #1704

Merged

giuli007 mentioned this pull request Jul 27, 2021

Apply picks up stale plans #1624

Closed

giuli007 requested a review from nishkrishnan July 28, 2021 17:13

github-actions bot added the Stale label Aug 28, 2021

github-actions bot closed this Sep 2, 2021

jamengual reopened this Jan 18, 2022

github-actions bot removed the Stale label Jan 19, 2022

github-actions bot added the Stale label Feb 18, 2022

github-actions bot closed this Feb 23, 2022

jamengual reopened this May 5, 2022

github-actions bot added the Stale label Jul 30, 2022

github-actions bot removed the Stale label Jul 31, 2022

github-actions bot added the Stale label Aug 30, 2022

github-actions bot removed the Stale label Aug 31, 2022

giuli007 force-pushed the delete_potentially_stale_previous_plans_which_fixes_1624 branch from 4e73bfe to 55a37bd Compare September 6, 2022 17:07

giuli007 added 4 commits September 15, 2022 10:05

Also delete locks along with plans when running

da060e7

autoplan or generic plan command

Fix Plan deletion tests after 0.19.8 rebase

07aa6f2

Fix Plan deletion tests after v0.19.9-pre.20220912 rebase

b3ed274

giuli007 force-pushed the delete_potentially_stale_previous_plans_which_fixes_1624 branch from 55a37bd to b3ed274 Compare September 15, 2022 17:25

jamengual reviewed Sep 15, 2022

View reviewed changes

Rename struct field to follow camelCase naming convention

67aa00a

giuli007 requested review from jamengual and nishkrishnan and removed request for nishkrishnan and jamengual September 16, 2022 15:36

lilincmu approved these changes Sep 23, 2022

View reviewed changes

lilincmu merged commit 2cd8bd9 into runatlantis:master Sep 23, 2022

jamengual mentioned this pull request Sep 23, 2022

Atlantis apply all after a failed apply; outputs Ran Apply for 0 projects #773

Open

Fauzyy mentioned this pull request Nov 10, 2022

Add the ability to ignore older plans that are no longer part of the current diff #2608

Closed

1 task

nitrocode added this to the v0.19.9 milestone Jan 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Delete previous plans on autoplan or atlantis plan #1633

Delete previous plans on autoplan or atlantis plan #1633

giuli007 commented Jun 15, 2021 •

edited

Loading

nishkrishnan left a comment

nishkrishnan Jun 22, 2021

giuli007 Jun 25, 2021 •

edited

Loading

giuli007 commented Jun 30, 2021 •

edited

Loading

giuli007 commented Jul 14, 2021

srlightbody commented Jul 21, 2021

angeloskaltsikis commented Jul 23, 2021

giuli007 commented Jul 27, 2021 •

edited

Loading

github-actions bot commented Aug 28, 2021

giuli007 commented Sep 14, 2021

giuli007 commented Oct 6, 2021

github-actions bot commented Feb 18, 2022

zraider7 commented May 5, 2022

jamengual commented May 5, 2022

github-actions bot commented Jul 30, 2022

okgolove commented Jul 30, 2022

github-actions bot commented Aug 30, 2022

giuli007 commented Aug 30, 2022

giuli007 commented Sep 6, 2022 •

edited

Loading

giuli007 commented Sep 15, 2022

jamengual Sep 15, 2022

giuli007 Sep 16, 2022

lilincmu left a comment

jamengual commented Sep 23, 2022

giuli007 commented Sep 29, 2022

	A `terraform plan` (without flags), like autoplans, discards all plans previously created with `atlantis plan` `-p`/`-d`/`-w`
	An `atlantis plan` (without flags), like autoplans, discards all plans previously created with `atlantis plan` `-p`/`-d`/`-w`

Delete previous plans on autoplan or atlantis plan #1633

Delete previous plans on autoplan or atlantis plan #1633

Conversation

giuli007 commented Jun 15, 2021 • edited Loading

nishkrishnan left a comment

Choose a reason for hiding this comment

nishkrishnan Jun 22, 2021

Choose a reason for hiding this comment

giuli007 Jun 25, 2021 • edited Loading

Choose a reason for hiding this comment

giuli007 commented Jun 30, 2021 • edited Loading

giuli007 commented Jul 14, 2021

srlightbody commented Jul 21, 2021

angeloskaltsikis commented Jul 23, 2021

giuli007 commented Jul 27, 2021 • edited Loading

github-actions bot commented Aug 28, 2021

giuli007 commented Sep 14, 2021

giuli007 commented Oct 6, 2021

github-actions bot commented Feb 18, 2022

zraider7 commented May 5, 2022

jamengual commented May 5, 2022

github-actions bot commented Jul 30, 2022

okgolove commented Jul 30, 2022

github-actions bot commented Aug 30, 2022

giuli007 commented Aug 30, 2022

giuli007 commented Sep 6, 2022 • edited Loading

giuli007 commented Sep 15, 2022

jamengual Sep 15, 2022

Choose a reason for hiding this comment

giuli007 Sep 16, 2022

Choose a reason for hiding this comment

lilincmu left a comment

Choose a reason for hiding this comment

jamengual commented Sep 23, 2022

giuli007 commented Sep 29, 2022

giuli007 commented Jun 15, 2021 •

edited

Loading

giuli007 Jun 25, 2021 •

edited

Loading

giuli007 commented Jun 30, 2021 •

edited

Loading

giuli007 commented Jul 27, 2021 •

edited

Loading

giuli007 commented Sep 6, 2022 •

edited

Loading