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

Manually planned projects/directories keep atlantis/apply check at 0/0 which allows merging without applying #3852

Open
nitrocode opened this issue Oct 12, 2023 · 8 comments
Labels
bug Something isn't working regression Bug introduced in a new version Stale

Comments

@nitrocode
Copy link
Member

nitrocode commented Oct 12, 2023

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request. Searching for pre-existing feature requests helps us consolidate datapoints for identical requirements into a single place, thank you!
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment.

Overview of the Issue

Prior to v0.26.0 and v0.25.0, atlantis would allow us to plan projects/directories manually and this would prevent the atlantis/apply check from being green until an atlantis apply was commented

image

Reproduction Steps

  1. Create a PR as a no-op PR that doesn't affect any projects
  2. Plan a project outside of the scope of the PR's file changes
  3. Notice that atlantis/plan is 1/1 and atlantis/apply is 0/0

Logs

Environment details

  • Atlantis version: v0.26.0
  • Deployment method: eks

Additional Context

@nitrocode nitrocode added bug Something isn't working regression Bug introduced in a new version labels Oct 12, 2023
@lukemassa
Copy link
Contributor

lukemassa commented Oct 13, 2023

I think it is definitely related, but I'm still trying to understand what's going on. This is what I'm seeing:

0.24.0:

When Plan check Apply Check
Created MR 0/0 00
manual unrelated plan with no change 1/1 0/0
manual unrelated plan with some change 1/1 0/0

0.25.0 (which introduced #3378 to mark as apply successful if nothing to do):

When Plan check Apply Check
Created MR 0/0 00
manual unrelated plan with no change 1/1 1/1
manual unrelated plan with some change 1/1 0/1

0.26.0 (where I added introduced #3747 to prevent gitlab from being stuck in pending state):

When Plan check Apply Check
Created MR 0/0 00
manual unrelated plan with no change 1/1 1/1
manual unrelated plan with some change 1/1 0/0

@nitrocode what do you expect to see? Semantically, given #3378, where we are now seems correct to me: after the plan where changes were introduced, we have done 1/1 plans and, having not attempted any applies, we've done 0/0 of them.

It might be a subtle gitlab vs github thing; in gitlab "atlantis/apply" shows up from the very beginning, as green. Is that not true in github?

@nitrocode
Copy link
Member Author

Thanks @lukemassa for looking into this and providing all the necessary PRs.

It might be a subtle gitlab vs github thing; in gitlab "atlantis/apply" shows up from the very beginning, as green. Is that not true in github?

I think this is the difference. In github, the atlantis/apply does not show up as green and needs to be turned green once an atlantis apply comment is applied which then allows the PR to get merged.

@jamengual
Copy link
Contributor

@lukemassa do you think you could work on a fix for this? and if that is the case document the nuance between gitlab and github for the status too.

@lukemassa
Copy link
Contributor

I'm happy to work on this but I'm not sure yet what should be done. If I understand correctly, and if @nitrocode is indeed seeing this starting on 0.25.0, it seems like it might be a "feature not a bug" of #3378. That PR added logic that updated the apply check on the plan run if there were no changes, which I believe would cover this case. Short of reverting that feature, I'm not sure what could be done. I might be missing something though.

@jamengual
Copy link
Contributor

@chroju do you have any input on this?

@chroju
Copy link
Contributor

chroju commented Oct 19, 2023

Sorry, but I do not understand what this issue is aiming to resolve. I will only explain the changes made by #3378.

With the merger of #3378, running atlantis plan on a PR will now update the atlantis/apply status. In this case, if the number of PullStatus.Projects matches the number of projects for which apply is successful, or if the plan result is No Changes, then the atlantis/apply status will be considered successful. Therefore, if a atlantis plan is executed for a project that has not been modified by the PR, as @lukemassa wrote, the apply status will be 0/1. This would be pending status on GitHub.

I'm not familiar with the situation prior to v0.25.0. I believe that at that time, even if you ran atlantis plan, the apply status would not have been updated. Therefore, if a PR with no *.tf files changes was made, the atlantis/apply status would be green, and if a plan was executed, it should have continued to be green at 0/0.

@brandon-fryslie
Copy link

brandon-fryslie commented Jan 11, 2024

With the merger of #3378, running atlantis plan on a PR will now update the atlantis/apply status.

This is a huge mistake. It could be acceptable to automatically run apply if plan shows 0 changes, but plan doesn't contain any of the logic for apply and having atlantis show 0 changes on a plan is not equivalent to running a successful apply in quite a few more complicated environments. Look at how many bugs are caused by this, and this is just what is currently reported.

It feels strange to ask, but can we have a way to require Atlantis actually run apply? If I'm not mistaken there isn't actually any way to require apply to run before setting the atlantis/apply status to "success" anymore. I consider that a core feature of Atlantis.

@GenPage
Copy link
Member

GenPage commented Jan 26, 2024

With the merger of #3378, running atlantis plan on a PR will now update the atlantis/apply status.

This is a huge mistake. It could be acceptable to automatically run apply if the plan shows 0 changes, but plan doesn't contain any of the logic for apply, and having Atlantis show 0 changes on a plan is not equivalent to running a successful apply in quite a few more complicated environments. Look at how many bugs are caused by this, and this is just what is currently reported.

It feels strange to ask, but can we have a way to require Atlantis actually run apply? If I'm not mistaken there isn't actually any way to require apply to run before setting the atlantis/apply status to "success" anymore. I consider that a core feature of Atlantis.

Before considering any action, we need to understand the problem correctly. There are multiple. The ask to change #3378 is not related to this issue about allowing manual plan/applies of projects unrelated to the PR. It should be a separate issue referencing #3378.

This issue is that the apply status checks have different behavior in GitLab vs. GitHub. However, the logic for handling these checks is in the core event logic that affects both VCS providers. It might be best to investigate moving logic to their client implementations respectfully.

I want to advocate for not imposing an opinionated way people must run their workflows so we're not constantly ping-ponging on the issue. Workflows in GH vs GL are different and continues to bite us. @lukemassa @chroju @nitrocode Let's continue the discussion around the title of the issue, "Manually planned projects/directories keep atlantis/apply check at 0/0 which allows merging without applying".

I think it is definitely related, but I'm still trying to understand what's going on. This is what I'm seeing:

0.24.0:

When Plan check Apply Check
Created MR 0/0 00
manual unrelated plan with no change 1/1 0/0
manual unrelated plan with some change 1/1 0/0
0.25.0 (which introduced #3378 to mark as apply successful if nothing to do):

When Plan check Apply Check
Created MR 0/0 00
manual unrelated plan with no change 1/1 1/1
manual unrelated plan with some change 1/1 0/1
0.26.0 (where I added introduced #3747 to prevent gitlab from being stuck in pending state):

When Plan check Apply Check
Created MR 0/0 00
manual unrelated plan with no change 1/1 1/1
manual unrelated plan with some change 1/1 0/0
@nitrocode what do you expect to see? Semantically, given #3378, where we are now seems correct to me: after the plan where changes were introduced, we have done 1/1 plans and, having not attempted any applies, we've done 0/0 of them.

It might be a subtle gitlab vs github thing; in gitlab "atlantis/apply" shows up from the very beginning, as green. Is that not true in github?

This is a good summary by Luke. Do we need to compensate somewhere in GH for the fact the atlantis/apply is not immediately available in GH like in GL?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working regression Bug introduced in a new version Stale
Projects
None yet
Development

No branches or pull requests

6 participants