-
Notifications
You must be signed in to change notification settings - Fork 61
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: Prevent deletes in consumers and streams #89
feat: Prevent deletes in consumers and streams #89
Conversation
3e4e803
to
38a3517
Compare
967a702
to
33a94a9
Compare
Adding a `preventDelete` property for streams and consumers CRDS which when set to true nack should be able to delete it, otherwise it should just ignore that stream or consumer. The use case for this is in a multi-cloud context when we have deploy deployed in different clouds it becomes troublesome. For example, nack on GKE might check that a stream CRD is missing and proceeds to delete it, however that CRD exists in AWS and AKS and it should not be deleted because its going to impact other applications.
33a94a9
to
d1e9710
Compare
Managed Resource Deletion has historically been handled in Finalizers in Operators, but in NACK is being handled in a list/clean loop that was implemented in #56 I have opened #90 to try to figure out if we should go back to a Finalizer approach for resource deletion If we used finalizers, then only a Deleted CRD Finalizer would be able to delete a Managed NATS Stream or Consumer, so you wouldn't get any ghost deletes from a missing CRD |
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.
Wording in the descriptions is backwards, I've suggested some wording that should be a little clearer
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.
I updated the descriptions, I think this looks good now. Would like to get @wallyqs opinion on it too though
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.
LGTM
Adding a
preventDelete
property for streams and consumers which when falsenack should be able to delete it, otherwise it should just ignore that
stream or consumer.
The use case for this is in a multi-cloud context when we have deploy
deployed in different clouds it becomes troublesome. For example, nack
on GKE might check that a stream CRD is missing and proceeds to delete
it, however that CRD exists in AWS and AKS and it should not be deleted
because its going to impact other applications.