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

DestroyAll only works if CreateAll or LoadState were called first #37

Closed
catsby opened this issue Jun 11, 2021 · 3 comments · Fixed by #52
Closed

DestroyAll only works if CreateAll or LoadState were called first #37

catsby opened this issue Jun 11, 2021 · 3 comments · Fixed by #52
Assignees
Labels
bug Something isn't working resource/manager

Comments

@catsby
Copy link
Contributor

catsby commented Jun 11, 2021

DestroyAll checks for Order or createState to be set before attempting to destroy anything, which are only(?) set when CreateAll (or LoadState) is called first. I believe this prevents "manual" cleanup like seen in docker/platform.go and k8s/platform.go. I attempt to replicate this style of deleting resources by setting their state in hashicorp/waypoint#1648 but the corresponding Destroy methods are never called

@catsby catsby changed the title DestroyAll only works if CreateAll was called first DestroyAll only works if CreateAll or LoadState were called first Jun 11, 2021
@mitchellh
Copy link
Contributor

This should be easy to write a unit test for.

We need state to do a destroy, so it makes sense state needs to be set. But if there are cases you can set state and destroy still isn't called, then its definitely a bug.

@catsby
Copy link
Contributor Author

catsby commented Jun 11, 2021

@mitchellh I believe #38 reproduces this, or at least it does in so far as trying to use SetState followed by DestroyAll as shown in the Docker/K8s platform files

@briancain briancain added bug Something isn't working resource/manager labels Jun 14, 2021
@catsby
Copy link
Contributor Author

catsby commented Jun 16, 2021

Mitchell mentioned he wanted to look into this so I'm going to un-assign myself at this time.

@catsby catsby removed their assignment Jun 16, 2021
mitchellh added a commit that referenced this issue Aug 10, 2021
Fixes #37

This calls Destroy properly on a resource if SetState is manually
called. There is a big limitation here in that we don't know the exact
proper destroy order to call, but argmapper gets us most likely correct
due to state dependencies. And, as the comment in the code states, the
ordering doesn't matter for any practical use case we have today since
all manual SetStates today are single resources. Hopefully we switch
everything to resource manager soon so this doesn't matter.
mitchellh added a commit to hashicorp/waypoint that referenced this issue Aug 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working resource/manager
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants