-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
feat: state rm #2880
feat: state rm #2880
Conversation
5bcb7c7
to
91f4995
Compare
``` | ||
### Explanation | ||
Runs `terraform state rm` that matches the directory/project/workspace. | ||
This command discards the terraform plan result. After an import and before an apply, another `atlantis plan` must be run again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there be another flag like atlantis terraform state rm ADDRESS --auto-plan-on-success
?
We spoke about it in another comment for atlantis terraform import
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it might be helpful. In this case, we can do atlantis plan
manually, so this is not a required parameter.
server/controllers/events/testfixtures/test-repos/state-rm-multiple-project/exp-output-plan.txt
Outdated
Show resolved
Hide resolved
...rollers/events/testfixtures/test-repos/state-rm-single-project/exp-output-import-foreach.txt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for this contribution. Looks very promising!
da8a8b1
to
410920e
Compare
...controllers/events/testfixtures/test-repos/state-rm-multiple-project/exp-output-autoplan.txt
Outdated
Show resolved
Hide resolved
...controllers/events/testfixtures/test-repos/state-rm-multiple-project/exp-output-autoplan.txt
Outdated
Show resolved
Hide resolved
|
stateRmCmd = append(stateRmCmd, ctx.EscapedCommentArgs...) | ||
out, err := p.TerraformExecutor.RunCommandWithVersion(ctx, filepath.Clean(path), stateRmCmd, envs, tfVersion, ctx.Workspace) | ||
|
||
// If the state rm was successful and a plan file exists, delete the plan. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should document this in the .md docs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docs is written in https://deploy-preview-2880--runatlantis.netlify.app/docs/using-atlantis.html#atlantis-state-rm, like This command discards the terraform plan result. After run state rm and before an apply, another atlantis plan must be run again.
In addition, I added vcs comment about discarding like notify import/state rm discard plan file
revert to 🔁 use repeat instead warning for re-plan
@@ -217,7 +225,7 @@ func (p *DefaultProjectCommandBuilder) BuildAutoplanCommands(ctx *command.Contex | |||
// See ProjectCommandBuilder.BuildPlanCommands. | |||
func (p *DefaultProjectCommandBuilder) BuildPlanCommands(ctx *command.Context, cmd *CommentCommand) ([]command.ProjectContext, error) { | |||
if !cmd.IsForSpecificProject() { | |||
return p.buildAllCommandsByCfg(ctx, cmd.CommandName(), cmd.Flags, cmd.Verbose) | |||
return p.buildAllCommandsByCfg(ctx, cmd.CommandName(), cmd.SubName, cmd.Flags, cmd.Verbose) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably came up before, but why not just pass the whole cmd
instead of select SubName
, Flags
, and Verbose
like we do with buildProjectCommand
and buildProjectApplyCommand
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BuildAutoplanCommands
doesn't have cmd *CommentCommand
. We can do like following, but I think this should not be introduced. Because cmd := CommentCommand{Name: command.Plan}
might be cause nil pointer panic or invalid implementations by contributers who want to change buildAllCommandsByCfg
and don't know passed lacked CommentCommand
.
> gd
diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go
index d1068da2..08928d88 100644
--- a/server/events/project_command_builder.go
+++ b/server/events/project_command_builder.go
@@ -207,7 +207,8 @@ type DefaultProjectCommandBuilder struct {
// See ProjectCommandBuilder.BuildAutoplanCommands.
func (p *DefaultProjectCommandBuilder) BuildAutoplanCommands(ctx *command.Context) ([]command.ProjectContext, error) {
- projCtxs, err := p.buildAllCommandsByCfg(ctx, command.Plan, "", nil, false)
+ cmd := CommentCommand{Name: command.Plan}
+ projCtxs, err := p.buildAllCommandsByCfg(ctx, cmd)
if err != nil {
return nil, err
}
@@ -225,7 +226,7 @@ func (p *DefaultProjectCommandBuilder) BuildAutoplanCommands(ctx *command.Contex
// See ProjectCommandBuilder.BuildPlanCommands.
func (p *DefaultProjectCommandBuilder) BuildPlanCommands(ctx *command.Context, cmd *CommentCommand) ([]command.ProjectContext, error) {
if !cmd.IsForSpecificProject() {
- return p.buildAllCommandsByCfg(ctx, cmd.CommandName(), cmd.SubName, cmd.Flags, cmd.Verbose)
+ return p.buildAllCommandsByCfg(ctx, cmd)
}
@@ -12,5 +12,5 @@ The resources that were imported are shown above. These resources are now in | |||
your Terraform state and will henceforth be managed by Terraform. | |||
``` | |||
|
|||
* :repeat: To **plan** this project again, comment: | |||
* :repeat: plan file was discarded, to **plan** this project again, comment: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like more of a summary statement. Generally we put summary statements on its own line between the triple backticks and the * :repeat:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
like this? perl -pi -e 's!* 🔁 plan file was discarded. to!🚮 A plan file was dis…
your Terraform state and will henceforth be managed by Terraform.
```
:put_litter_in_its_place: A plan file was discarded. Re-plan would be required before applying.
* :repeat: To **plan** this project again, comment:
* `atlantis plan`
…carded. Re-plan would be required before applying.\n\n\* 🔁 To!g' server/**/*
@krrrr38 is this ready to be merged? |
yes |
Thanks @krrrr38 ! Can't wait to try out this feature. |
Can the remove behavior be overridden via custom workflows? Those of us who use Terragrunt would love to get this feature too. |
@gmaghera you can customize state rm workflow by overriing In |
what
atlantis state rm ADDRESS...
commandstate_rm
stage which doinit
andstate_rm
stepsstate_rm
step doterraform state rm
and discard a planfile.state_requirements
likeapply_requirements
why
terraform state rm
is required manually. If Atlantis provides this feature, we do it on GitHub PR and so on.references
atlantis state rm
subcommand #273