Fix: obsolete terraform flags #2138
Conversation
Terraform directory parameter has to be specified via optional -chdir flag. This commit removed the explicit '.' which is invalid since terraform 1.0.0
Terraform provides detailed status code which can be used in plan commands to determine if plan contains any changes. This commit adds explicit status check to before sending the results for parsing
|
/cla sign |
|
Thank you for signing the CLA. Your signing has been successfully committed. |
|
|
||
| func GetExitCode(err error) int { | ||
| if err == nil { | ||
| return 0 |
There was a problem hiding this comment.
I wonder which value is better to set here. 0 or 2.
Because we are using the -detailed-exitcode flag, so with the new version of terraform, we expect err will not be nill, right?
So IMO, when err is nil, returning 2 is safer than 0.
How do you think about this?
There was a problem hiding this comment.
even with -detailed-exitcode the err will be nil if the plan commands succeeds and produces plan with no changes.
-detailed-exitcode only gives us ability to differentiate between actual plan error (status code 1) and plan witch changes (status code 2).
So I would keep this as is
There was a problem hiding this comment.
Thanks for your explaination.
I got your point. 💯
|
@p0tr3c Hello. Thank you for your pull request. We really appreciate this. |
|
/lgtm |
|
/trigger presubmits |
|
@nghialv: Your requested presubmits has been scheduled in response to this comment. |
|
Code coverage for golang is
|
|
Thx |
|
This pull request is APPROVED by Approvers can cancel the approval by writing |
|
@p0tr3c Hello, I just want to say that your patch has been released recently. Btw, if you are using terraform our new feature Plan Preview may be useful too. Thanks. |
|
Yeah, really like the idea of PipeCD. Hopefully will find some more time to contribute. |
|
It's great to hear that. |
4 participants
What this PR does / why we need it:
Terraform cloudprovider failed to execute with the latest terraform stable version 1.0.0. This PR addresses the issues caused by changes to command line flags, and
The directory parameter '.' has been changed to
-chdirand is optional. This PR removes the flag from the commandThe
-lock=falseis no longer valid parameter forinitcommand. This PR removes the flag from the command.I've tested the changed parameters with following terraform versions:
Parsing plan with no changes failed in latest terraform version. This is most likely due to subtle changes to the text output.
This PR proposes change to detect explicit exit status to parse only plans with changes. This method should be less error prone when the string output of terraform changes.
Which issue(s) this PR fixes:
Does this PR introduce a user-facing change?: