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

Digger reports success even though Terraform apply fails #1658

Closed
peteher opened this issue Aug 12, 2024 · 9 comments · Fixed by #1760
Closed

Digger reports success even though Terraform apply fails #1658

peteher opened this issue Aug 12, 2024 · 9 comments · Fixed by #1760

Comments

@peteher
Copy link

peteher commented Aug 12, 2024

Problem:

Terraform apply reports error/exit code 1, but pipeline won't fail as Digger saying "success"

Setup:

  • no-backend setup
  • terraform 1.5.7
  • digger vLatest today - v0.6.33
  • upload-plan-destination: github
  • digger.yaml:
projects:
  - name: default
    workflow: my_custom_workflow
workflows:
  my_custom_workflow:
    workflow_configuration:
      on_pull_request_pushed: [ digger plan ]
      on_pull_request_closed: [ digger unlock ]
      on_commit_to_default: [ digger apply ]
auto_merge: false
collect_usage_data: false

Flow:

  1. changes delivered to branch, PR (ie tag resources)
  2. action runs to "plan" for PR
  3. plan.out attached to action
  4. make change on infra via console for eg delete a resource that the plan wants to tag
  5. merge PR to "apply"
  6. Terraform apply will fail as step 1 change can't be committed on step 4 removed resource
  7. Digger would say "apply failed, skipping job" and "success" - pipeline finishing green ✅ - I'd expect ❌

image

Using version v0.3.22 I get the expected fail:
image

@motatoes
Copy link
Contributor

Hi @peteher thanks for reporting this, might be related to #1579 as well

But it is backendless so I believe this is a slightly different issue for run Jobs

@Kamalesh-Seervi
Copy link

can I work on this issue or its solved

@peteher
Copy link
Author

peteher commented Sep 16, 2024

Hi @Kamalesh-Seervi
I wonder if you managed to check on this issue, maybe made some progress already?

Current workaround used is that I itemize the steps of a workflow's apply step as:

workflows:
  my_custom_workflow:
    apply:
      steps:
        - apply
        - run: echo "completed" > /tmp/digger_output.txt

Even though Digger reports "success" it will not execute the run step after the apply step (like it does if the apply really worked):

Error: Saved plan is stale
The given plan file can no longer be applied because the state was changed by
another operation after the plan was created.
Error: exit status 1
Failed to Run digger apply command. error executing apply: exit status 1
error while running command digger apply for project default: Failed to run digger apply command. <nil>
Project default command digger apply failed, skipping job
Commands executed successfully
Digger finished successfully

And so I can make a match from the pipeline to see if the run has completed or not.

@fleroux514
Copy link
Contributor

Just noticed this issue in v0.6.44.

To me this is a blocker since it would allow someone to merge a PR even tough the apply is failing. Preventing this use case is the core purpose of digger so I don't understand that it hasn't been addressed since v0.6.33.

It would be nice if there would be a blocker tag for issues that must be fixed in upcoming releases, before even thinking of adding new functionality. Bugs in new functionality is understandable but regressions are not acceptable.

@motatoes
Copy link
Contributor

Hi folks just realised this issue made it to the top upvotes, I didn't realise it was an issue of high severity. Thanks @fleroux514 for the blocker idea I think that will adopt this strategy. I will try to take a look at this soon

@motatoes
Copy link
Contributor

For some reason when I read this issue previously I thought it was related to the one I linked of posting the error back to the comment, did not realise that the pipeline was succeeding

@peteher
Copy link
Author

peteher commented Sep 27, 2024

@motatoes I tested and found it's been an issue since at least v0.5.x - was working back with v0.3.22.

Had to update to vLatest though for OpenTofu to work, however in order to catch whether a digger apply failed or not in a pipeline, the noted workaround is required at the moment.

minamijoyo added a commit to minamijoyo/digger that referenced this issue Oct 10, 2024
Fixes diggerhq#1658

Error handling in backendless mode was incorrect, resulting in the
workflow exit status to be success even if the command execution failed.
This patch addresses this issue.

The following are some additional notes of this fix:

- When `allAppliesSuccessful` is false, we need to call
  `ReportErrorAndExit()` to ensure that the workflow fails with a
  non-zero exit status.
- Since `ReportErrorAndExit()` invokes `os.Exit()` immediately, it
  should be called after setting the status of the pull request.
- Contrary to intuition, `atLeastOneApply` is counted even if plan
  command, so we need to use `scheduler.IsPlanJobs()` to determine if
  the current workflow is plan or apply.
minamijoyo added a commit to minamijoyo/digger that referenced this issue Oct 10, 2024
Fixes diggerhq#1658

Error handling in backendless mode was incorrect, resulting in the
workflow exit status to be success even if the command execution failed.
This patch addresses this issue.

The following are some additional notes of this fix:

- When `allAppliesSuccessful` is false, we need to call
  `ReportErrorAndExit()` to ensure that the workflow fails with a
  non-zero exit status.
- Since `ReportErrorAndExit()` invokes `os.Exit()` immediately, it
  should be called after setting the status of the pull request.
- Contrary to intuition, `atLeastOneApply` is counted even if plan
  command, so we need to use `scheduler.IsPlanJobs()` to determine if
  the current workflow is plan or apply.
@minamijoyo
Copy link
Contributor

@motatoes This issue is a really blocker for me using Digger in production, so I fixed it in #1760. Please review 🙏

@minamijoyo
Copy link
Contributor

Fixed in v0.6.50 🚀

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 a pull request may close this issue.

5 participants