fix exit code for each exec command #1065
Conversation
tests/fixtures/scenarios/exitCode/components/terraform/empty/variables.tf
Outdated
Show resolved
Hide resolved
📝 WalkthroughWalkthroughThis pull request enhances error handling in a Go utility by introducing logic that distinguishes exit errors from generic errors, allowing the application to exit with the appropriate code. In addition, new YAML configuration files define a Terraform deployment scenario with associated stack components and logging settings. New Terraform configuration simulates command exit codes, and updated test cases verify that the atmos command returns expected exit codes under various scenarios. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Atmos Command
participant Func as PrintErrorMarkdownAndExit
participant ExecErr as *exec.ExitError
Caller->>Func: Call PrintErrorMarkdownAndExit(title, error, suggestion)
alt error is of type *exec.ExitError
Func->>ExecErr: Retrieve exit code from error
Func->>Caller: Exit with error's exit code
else error is not *exec.ExitError
Func->>Caller: Exit with default code 1
end
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (5)
🔇 Additional comments (4)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
While we are at it, we should also test that the failed terraform apply also exits 1, and that a successful apply exits zero. |
Done But a note now tests will pass only first time. I need to delete the resources so that they always, pass. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/fixtures/scenarios/exitCode/components/terraform/hook-and-store/main.tf(1 hunks)tests/test-cases/exec-command.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Analyze (go)
- GitHub Check: Summary
🔇 Additional comments (5)
tests/fixtures/scenarios/exitCode/components/terraform/hook-and-store/main.tf (3)
1-8: Provider Configuration Validated.
The new Terraform block correctly specifies thenullprovider with the appropriate source and version. This configuration should ensure that the required provider is fetched in line with the intended test scenarios.
10-14: Variable Declaration is Clear.
The "stage" variable is well-defined with a descriptive message and default value, making its purpose as a flag file obvious.
16-34: Valid Flag File Handling in Resource.
Thenull_resource "run_once"resource is implemented effectively:
- It uses a dynamic trigger (via
timestamp()) to re-run the provisioner.- The embedded shell script correctly checks for the existence of the file defined by the "stage" variable.
- Exiting with code 1 when the file exists aligns with the expected failure behavior to support exit code testing.
Everything looks in order from a configuration perspective.
tests/test-cases/exec-command.yaml (2)
1-16: Terraform Plan Exit Code Test is Good.
This test case verifies that the CLI correctly passes through an exit code of 2 when executing aterraform planwith detailed exit codes. The configuration here meets the PR objective of ensuring exit code consistency.
17-31: Successful Terraform Apply Test Confirmed.
The test case expecting an exit code of 0 for a successfulterraform applyis set up properly with the correct arguments and expected behavior.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@samtholiya let me know when the PR is good for the final review |
tests/fixtures/scenarios/exitCode/components/terraform/hook-and-store/main.tf
Outdated
Show resolved
Hide resolved
tests/fixtures/scenarios/exitCode/components/terraform/hook-and-store/main.tf
Outdated
Show resolved
Hide resolved
tests/fixtures/scenarios/exitCode/components/terraform/hook-and-store/main.tf
Outdated
Show resolved
Hide resolved
…d-store/main.tf Co-authored-by: Andriy Knysh <aknysh@users.noreply.github.com>
…d-store/main.tf Co-authored-by: Andriy Knysh <aknysh@users.noreply.github.com>
We support a new “clean” setting in the test configuration that will delete any unstated files inside the scenario, when set to true. Use this to automatically ensure the working directory is reset when you need it to be. |
210a477 to
6f6b236
Compare
…ttps://github.com/cloudposse/atmos into feature/dev-3051-fix-terraform-exit-code-mismatch
fd13d10 to
f04691a
Compare
tests/fixtures/scenarios/exitCode/components/terraform/hook-and-store/main.tf
Outdated
Show resolved
Hide resolved
tests/fixtures/scenarios/exitCode/components/terraform/hook-and-store/main.tf
Outdated
Show resolved
Hide resolved
…ttps://github.com/cloudposse/atmos into feature/dev-3051-fix-terraform-exit-code-mismatch
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/fixtures/scenarios/exitCode/stacks/stack.yaml (1)
13-13: Nitpick: Missing newline at end of file.
YAMLLint reported that there is no new line character at the end on line 13. Please add a final newline to ensure compliance with YAML formatting guidelines.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 13-13: no new line character at the end of file
(new-line-at-end-of-file)
tests/test-cases/exec-command.yaml (1)
19-36: Test Case for Exit Code (0) Verification.
The test forterraform applyon component1 is clearly set to expect an exit code of 0. Please remove the trailing spaces on line 34, as flagged by YAMLlint, to maintain clean formatting.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 34-34: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
tests/fixtures/scenarios/exitCode/components/terraform/exit-code/main.tf(1 hunks)tests/fixtures/scenarios/exitCode/stacks/stack.yaml(1 hunks)tests/test-cases/exec-command.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
tests/fixtures/scenarios/exitCode/stacks/stack.yaml
[error] 13-13: no new line character at the end of file
(new-line-at-end-of-file)
tests/test-cases/exec-command.yaml
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 52-52: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Analyze (go)
- GitHub Check: Summary
🔇 Additional comments (8)
tests/fixtures/scenarios/exitCode/stacks/stack.yaml (1)
1-12: Good configuration for exit codes.
The structure defines the Terraform exit-code components clearly with the appropriate metadata (usingcomponent: exit-code) and variables. The separation betweencomponent1(with only a stage) andcomponent2(with an explicitexit_code: 1) is clear and aligns with the intended test scenarios.tests/fixtures/scenarios/exitCode/components/terraform/exit-code/main.tf (4)
1-8: Terraform Provider Block Looks Good.
The provider declaration for thenullprovider from HashiCorp is correctly configured with a version constraint of "~> 3.0".
10-14: Variable 'stage' is Well-Defined.
Thestagevariable has a clear description and a default value of "test", which is appropriate for simulation in test scenarios.
16-20: Variable 'exit_code' is Correctly Set Up.
The definition of theexit_codevariable with type number and a default value of 0 is aligned with the requirements for simulating different exit states.
23-31: Null Resource for Exit Code Simulation is Appropriate.
Thenull_resourcenamedfail_on_second_applyis configured to always run (usingtimestamp()as a trigger), and the local-exec provisioner correctly executes a command that exits with the value defined byvar.exit_code.tests/test-cases/exec-command.yaml (3)
2-18: Test Case for Exit Code (2) Verification.
This test case forterraform planensures an exit code of 2 is returned when a diff is expected (using the-detailed-exitcodeflag). The inline comment clarifies the rationale for expecting exit code 2, which enhances clarity.
37-54: Test Case for Exit Code (1) Verification.
This test case for a failingterraform apply(using component2) correctly expects an exit code of 1. In addition to removing the trailing spaces on line 52, consider adding a brief inline comment explaining that the failure is due to the configuration in component2 (i.e. the variableexit_codebeing set to 1) for greater clarity.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 52-52: trailing spaces
(trailing-spaces)
1-55: Suggestion: Enhance Documentation in Test Cases.
Adding a header comment or more detailed inline notes at the top of this YAML file could help future maintainers quickly understand the differences between these test scenarios—especially clarifying whyterraform planreturns exit code 2 and how the various exit codes are simulated across the components.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 52-52: trailing spaces
(trailing-spaces)
tests/fixtures/scenarios/exitCode/components/terraform/exit-code/main.tf
Outdated
Show resolved
Hide resolved
…ttps://github.com/cloudposse/atmos into feature/dev-3051-fix-terraform-exit-code-mismatch
|
These changes were released in v1.163.0. |
what
why
Improved User Experience: It provides more accurate feedback to the user, reflecting the success or failure of the desired program.
Better Integration with Other Tools: Many CI/CD systems, monitoring tools, or scripts rely on exit codes to determine whether a task has succeeded or failed. Matching the exit codes allows seamless integration with existing workflows.
Debugging and Troubleshooting: By matching exit codes, users and developers can more easily trace issues and understand which program (or specific execution) caused the failure.
This change helps ensure the CLI behaves more predictably and conforms to standard practices for handling exit codes.
references
Summary by CodeRabbit
New Features
Tests