Skip to content
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

fix: successfully delete instances and bindings with invalid state #955

Merged
merged 1 commit into from
Feb 23, 2024

Conversation

blgm
Copy link
Member

@blgm blgm commented Feb 15, 2024

NOTE: please merge PR #954 first

As part of the upgrade epic, we added checks to make sure that service
instances and bindings had been updated to the latest version. This
involves parsing the Terraform state and extracting the version. But
sometimes the Terraform state is not valid due to being empty, or being
invalid JSON. Typically this is after a failure to provision/bind,
and the service instance or binding is in a failed state. In these
situations, the lack of state data means CSB does not have enough data to
successfully clean up using Terraform. We have had feedback that this
can be problematic for users because they have to do a manual purge of
the service instance, and it would be easier if the delete operations
would just work. This is a tradeoff because a manual purge could have
the effect of triggering someone to look at the backing IaaS to look for
resource leakage. By making this just work, we might make resource
leakage worse.

#185835561

As part of the upgrade epic, we added checks to make sure that service
instances and bindings had been updated to the latest version. This
involves parsing the Terraform state and extracting the version. But
sometimes the Terraform state is not valid due to being empty, or being
invalid JSON. Typically this is after a failure to provision/bind,
and the service instance or binding is in a failed state. In these
situations, the lack of state data means CSB does not have enough data to
successfully clean up using Terraform. We have had feedback that this
can be problematic for users because they have to do a manual purge of
the service instance, and it would be easier if the delete operations
would just work. This is a tradeoff because a manual purge could have
the effect of triggering someone to look at the backing IaaS to look for
resource leakage. By making this just work, we might make resource
leakage worse.

[#185835561](https://www.pivotaltracker.com/story/show/185835561)
@pivotal-marcela-campo
Copy link
Member

LGTM. There is one scenario missing however, as serviceProvider.CheckOperationConstraints would fail before if there is no deployment at all, and this indeed can happen . Will PR that check separately.

@pivotal-marcela-campo pivotal-marcela-campo merged commit aee6445 into main Feb 23, 2024
7 checks passed
@pivotal-marcela-campo pivotal-marcela-campo deleted the nofailondelete branch March 22, 2024 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

2 participants