Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

Clean-up when destroying environment #527

Merged
merged 6 commits into from
Feb 7, 2019
Merged

Clean-up when destroying environment #527

merged 6 commits into from
Feb 7, 2019

Conversation

MattiasGees
Copy link
Member

@MattiasGees MattiasGees commented Sep 17, 2018 •
edited

What this PR does / why we need it:
This will destroy a complete environment and all of its clusters:

  • Run terraform destroy
  • Remove state files from S3 and dynamodb
  • Move environment folder to .archive
  • Remove environment config from tarmak.yaml

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #
part of #523
Special notes for your reviewer:

Release note:

Add terraform environment destroy command

@jetstack-bot jetstack-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 17, 2018
MattiasGees
MattiasGees commented Sep 17, 2018
View reviewed changes
pkg/tarmak/terraform.go Outdated
@@ -179,6 +179,13 @@ func (c *CmdTerraform) Destroy() error {
return err
}

c.tarmak.cluster.Log().Info("Removing environment")

err = c.tarmak.environment.Remove()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if we have to make this command part of tarmak cluster destroy. Input is welcome for this one!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be a different command that will remove the environment and all clusters within from tarmak.yaml

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@jetstack-bot jetstack-bot added dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. and removed dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. labels Sep 18, 2018
@MattiasGees MattiasGees force-pushed the feature/destroy-state branch 2 times, most recently from 5585531 to 925be30 Compare September 18, 2018 07:13
@MattiasGees
Copy link
Member Author

/unassign
/assign @simonswine

@MattiasGees
Copy link
Member Author

/unassign @simonswine
/assign

@jetstack-bot jetstack-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 19, 2018
@MattiasGees MattiasGees changed the title Clean-up when destroying environment WIP: Clean-up when destroying environment Sep 20, 2018
@jetstack-bot jetstack-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 20, 2018
@jetstack-bot jetstack-bot added dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. and removed dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Sep 20, 2018
@jetstack-bot jetstack-bot added dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. and removed dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. labels Sep 20, 2018
@jetstack-bot jetstack-bot added dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. and removed dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. labels Sep 20, 2018
@jetstack-bot jetstack-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 28, 2018
@jetstack-bot jetstack-bot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Dec 6, 2018
@MattiasGees
Copy link
Member Author

/unassign @simonswine
/assign

Taking this back. Found a bug when destroying a multicluster. In the hub, it doesn't set the socket path for the Tarmak provider.

provider "tarmak" {
  socket_path = ""
}

@jetstack-bot jetstack-bot added the kind/documentation Categorizes issue or PR as related to documentation. label Dec 19, 2018
@MattiasGees
Copy link
Member Author

Fixed it
/unassign
/assign @simonswine

@jetstack-bot jetstack-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 23, 2019
@jetstack-bot jetstack-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 24, 2019
@jetstack-bot jetstack-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 2, 2019
@MattiasGees @simonswine
Add destroy environment command
f6b0e35 
This command will destroy all clusters in an environement, remove SSH and vault token and remove the environment config from tarmak.yaml

Signed-off-by: Mattias Gees <mattias.gees@gmail.com>
@MattiasGees @simonswine
FMT fix
3df54e8 
Signed-off-by: Mattias Gees <mattias.gees@gmail.com>
@MattiasGees @simonswine
Remove new line
b18667e 
Signed-off-by: Mattias Gees <mattias.gees@gmail.com>
@MattiasGees @simonswine
Add extra check
64560ff 
Signed-off-by: Mattias Gees <mattias.gees@gmail.com>
@MattiasGees @simonswine
Fix terraform problem when destroying multiclusters
d90249d 
Signed-off-by: Mattias Gees <mattias.gees@gmail.com>
@MattiasGees @simonswine
Fix docs
0e52b7a 
Signed-off-by: Mattias Gees <mattias.gees@gmail.com>
@jetstack-bot jetstack-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 7, 2019
@simonswine
Copy link
Contributor

/lgtm

@jetstack-bot jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Feb 7, 2019
@simonswine
Copy link
Contributor

/approve

@jetstack-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: MattiasGees, simonswine

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jetstack-bot jetstack-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 7, 2019
@jetstack-bot jetstack-bot merged commit a20c305 into jetstack:master Feb 7, 2019
@MattiasGees MattiasGees deleted the feature/destroy-state branch February 9, 2019 15:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@simonswine simonswine simonswine left review comments

@JoshVanL JoshVanL JoshVanL requested changes

Assignees

@simonswine simonswine

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. kind/documentation Categorizes issue or PR as related to documentation. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@MattiasGees @JoshVanL @simonswine @jetstack-bot