Verify previous Terraform state can be reused#1430
Verify previous Terraform state can be reused#1430NamelessOne91 wants to merge 1 commit intoSUSE:masterfrom
Conversation
|
Looks a nice improvement. |
676b625 to
877d38e
Compare
IIRC plan does automatically a refresh to check differences between deployed resources and what's in the state file ... but I may not RC 😄 I added it and refactored the code a bit to be more robust. |
There was a problem hiding this comment.
Pull request overview
This PR adds validation logic to ensure that previous Terraform state files can be safely reused before attempting to restore them. The implementation searches through previous builds to find a valid Terraform state and validates it using terraform plan before proceeding.
Changes:
- Replaces simple artifact restoration with a validation loop that checks multiple previous builds
- Validates Terraform state by running
terraform initandterraform planin a temporary directory - Falls back gracefully when no valid previous state is found
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| jenkins_pipelines/environments/uyuni-master-dev-acceptance-tests-code-coverage | Adds Terraform state validation logic with build history traversal |
| jenkins_pipelines/environments/common/pipeline.groovy | Adds identical Terraform state validation logic |
| jenkins_pipelines/environments/common/pipeline-salt-shaker.groovy | Adds identical Terraform state validation logic |
| jenkins_pipelines/environments/common/pipeline-reference.groovy | Adds identical Terraform state validation logic |
| jenkins_pipelines/environments/common/pipeline-reference-new.groovy | Adds identical Terraform state validation logic |
| jenkins_pipelines/environments/common/pipeline-build-validation.groovy | Adds identical Terraform state validation logic plus fixes a comment typo |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| if (planOutput == 0) { | ||
| echo "Terraform state from build ${previousBuild.number} is valid." | ||
| copyArtifacts projectName: currentBuild.projectName, selector: specific("${previousBuild.number}" |
There was a problem hiding this comment.
After validating the Terraform state, copyArtifacts is called again on line 93, but artifacts were already copied on line 82. This duplicate copy operation is unnecessary and inefficient. The line 93 copyArtifacts call should be removed since the artifacts have already been copied and validated.
| copyArtifacts projectName: currentBuild.projectName, selector: specific("${previousBuild.number}" |
|
|
||
| if (planOutput == 0) { | ||
| echo "Terraform state from build ${previousBuild.number} is valid." | ||
| copyArtifacts projectName: currentBuild.projectName, selector: specific("${previousBuild.number}" |
There was a problem hiding this comment.
After validating the Terraform state, copyArtifacts is called again on line 51, but artifacts were already copied on line 40. This duplicate copy operation is unnecessary and inefficient. The line 51 copyArtifacts call should be removed since the artifacts have already been copied and validated.
| copyArtifacts projectName: currentBuild.projectName, selector: specific("${previousBuild.number}" |
| break | ||
| } else { | ||
| echo "Terraform state from build ${previousBuild.number} is invalid. Searching for another build." | ||
| foundState = false |
There was a problem hiding this comment.
The variable 'foundState' is undefined. This should be 'found' to match the variable declared on line 72. This will cause a runtime error as 'foundState' was never declared.
| foundState = false | |
| found = false |
| break | ||
| } else { | ||
| echo "Terraform state from build ${previousBuild.number} is invalid. Searching for another build." | ||
| foundState = false |
There was a problem hiding this comment.
The variable 'foundState' is undefined. This should be 'found' to match the variable declared on line 31. This will cause a runtime error as 'foundState' was never declared.
| foundState = false | |
| found = false |
| break | ||
| } else { | ||
| echo "Terraform state from build ${previousBuild.number} is invalid. Searching for another build." | ||
| foundState = false |
There was a problem hiding this comment.
The variable 'foundState' is undefined. This should be 'found' to match the variable declared on line 30. This will cause a runtime error as 'foundState' was never declared.
| foundState = false | |
| found = false |
|
|
||
| if (planOutput == 0) { | ||
| echo "Terraform state from build ${previousBuild.number} is valid." | ||
| copyArtifacts projectName: currentBuild.projectName, selector: specific("${previousBuild.number}" |
There was a problem hiding this comment.
After validating the Terraform state, copyArtifacts is called again on line 51, but artifacts were already copied on line 40. This duplicate copy operation is unnecessary and inefficient. The line 51 copyArtifacts call should be removed since the artifacts have already been copied and validated.
| copyArtifacts projectName: currentBuild.projectName, selector: specific("${previousBuild.number}" |
| break | ||
| } else { | ||
| echo "Terraform state from build ${previousBuild.number} is invalid. Searching for another build." | ||
| foundState = false |
There was a problem hiding this comment.
The variable 'foundState' is undefined. This should be 'found' to match the variable declared on line 61. This will cause a runtime error as 'foundState' was never declared.
| foundState = false | |
| found = false |
|
|
||
| if (planOutput == 0) { | ||
| echo "Terraform state from build ${previousBuild.number} is valid." | ||
| copyArtifacts projectName: currentBuild.projectName, selector: specific("${previousBuild.number}" |
There was a problem hiding this comment.
After validating the Terraform state, copyArtifacts is called again on line 82, but artifacts were already copied on line 71. This duplicate copy operation is unnecessary and inefficient. The line 82 copyArtifacts call should be removed since the artifacts have already been copied and validated.
| copyArtifacts projectName: currentBuild.projectName, selector: specific("${previousBuild.number}" |
| break | ||
| } else { | ||
| echo "Terraform state from build ${previousBuild.number} is invalid. Searching for another build." | ||
| foundState = false |
There was a problem hiding this comment.
The variable 'foundState' is undefined. This should be 'found' to match the variable declared on line 47. This will cause a runtime error as 'foundState' was never declared.
| foundState = false | |
| found = false |
|
|
||
| if (planOutput == 0) { | ||
| echo "Terraform state from build ${previousBuild.number} is valid." | ||
| copyArtifacts projectName: currentBuild.projectName, selector: specific("${previousBuild.number}" |
There was a problem hiding this comment.
After validating the Terraform state, copyArtifacts is called again on line 68, but artifacts were already copied on line 57. This duplicate copy operation is unnecessary and inefficient. The line 68 copyArtifacts call should be removed since the artifacts have already been copied and validated.
| copyArtifacts projectName: currentBuild.projectName, selector: specific("${previousBuild.number}" |
Related https://github.com/SUSE/spacewalk/issues/25242
Introduces additional checks when "use_previous_terraform_state"is checked, with the aim to validate a previous Terraform state is indeed present and can be safely reused.