Delete previous plans on autoplan or atlantis plan

[giuli007]

This PR aims to fix https://github.com/runatlantis/atlantis/issues/1624 (and https://github.com/runatlantis/atlantis/issues/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.

[giuli007]

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]

@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 https://github.com/runatlantis/atlantis/pull/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 https://github.com/runatlantis/atlantis/pull/1704 instead, let me know what you think.

[srlightbody]

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

[angeloskaltsikis]

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 https://github.com/runatlantis/atlantis/pull/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]

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 https://github.com/runatlantis/atlantis/pull/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 https://github.com/runatlantis/atlantis/pull/1399 and https://github.com/runatlantis/atlantis/issues/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[bot]]

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]

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

[giuli007]

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[bot]]

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]

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

[jamengual]

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

[giuli007]

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

@jamengual thanks. I need to find some time to try it out but I am interested in getting this updated! Thanks for reopening

[github-actions[bot]]

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]

I notice that the PR alternative to this one ( https://github.com/runatlantis/atlantis/pull/1704) has been merged but I also notice a different non-merged and currently closed PR (https://github.com/runatlantis/atlantis/pull/2316) to revert it because it looks like it is causing some issues. It is worth noting both these PRs where branched off version 0.17.0.

I still think the behaviour of this PR is more desirable (automatic deletion of plans instead of deletion of the cloned project directories) and it might in fact cause less issues.

Because both PRs were created quite some time ago and the codebase has evolved quite a bit (0.17.0 -> 0.19.4) aside from resolving conflicts they will need some testing to make sure the behaviour is still correct.

I still haven't managed to find time to do that but I'll try again soon.

[jamengual]

@giuli007 the closed pr was closed by mistake and since then was reverted and pushed to the prerelease.

you might want to pull and then we can see if we can get a review and merge this one after the next release

[github-actions[bot]]

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]

No stale please 🙈

[github-actions[bot]]

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]

no stale

[giuli007]

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 https://github.com/runatlantis/atlantis/pull/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.

[giuli007]

I've rebased on top of master / v0.19.9-pre.20220912 to resolve conflicts with the changes from https://github.com/runatlantis/atlantis/pull/2491

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

[jamengual]

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

[giuli007]

Thanks @jamengual @lilincmu