Space: improve destroy command #362
Conversation
AlainRoy
changed the title
cmd/up/space/destroy.go
Outdated
|
|
||
| commonParams | ||
|
|
||
| Confirmed bool `name:"yes-really-delete-spaces-and-all-data" type:"bool" help:"Bypass safety checks and destroy Spaces"` |
There was a problem hiding this comment.
Well, I love it, but it probably should be singular -- space instead of spaces.
There was a problem hiding this comment.
I personally like force since it follows other conventions 🤷
There was a problem hiding this comment.
Well I like force and it does follow other conventions, I wanted to make people think.
|
Do we want to pipe in allowing them to orphan their resources? |
| if err := c.mgr.Uninstall(); err != nil { | ||
| return err | ||
| } | ||
| return c.kClient.CoreV1().Namespaces().Delete(context.Background(), nsUpboundSystem, v1.DeleteOptions{}) |
There was a problem hiding this comment.
This would delete the crossplane and upbound-pull-secret that we ask users to create before deploying. Is that OK? I could at least see it making testing more tedious. Maybe we could introduce a flag to skip deleting the namespace?
There was a problem hiding this comment.
Check the latest push -- we only delete the namespace if there are no orphans.
Yeah, implemented in the last push. The way I did it was to run not execute any hooks when orphan is set. The other way would be to upgrade and set the deletionPolicy, however that doesn't work with out a token file; it seemed crazy to require a token file to remove something. Since the delete is part of a hook, disabling the hook accomplished the same goal. If you |
|
@tnthornton @branden if the last drop looks good, go ahead and merge. |
Description of your changes
For Spaces destroy command:
upbound-systemnamespace on destroyI have:
make reviewableto ensure this PR is ready for review.backport release-x.ylabels to auto-backport this PR, as appropriate.How has this code been tested
Tested against v0.15.1 with Spaces