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

feat: Prevent deletes in consumers and streams #89

Merged

Conversation

danielcibrao-form3
Copy link
Contributor

@danielcibrao-form3 danielcibrao-form3 commented Oct 26, 2022

Adding a preventDelete property for streams and consumers which when false
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.

@danielcibrao-form3 danielcibrao-form3 force-pushed the dc-feat-deletable-resource branch 3 times, most recently from 3e4e803 to 38a3517 Compare October 26, 2022 15:53
@danielcibrao-form3 danielcibrao-form3 changed the title feat: Mark a consumer or stream has not deletable feat: Prevent deletes in consumers and streams Oct 26, 2022
@danielcibrao-form3 danielcibrao-form3 force-pushed the dc-feat-deletable-resource branch 3 times, most recently from 967a702 to 33a94a9 Compare October 26, 2022 15:57
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.
@caleblloyd
Copy link
Contributor

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

Copy link
Contributor

@caleblloyd caleblloyd left a 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

deploy/crds.yml Outdated Show resolved Hide resolved
deploy/crds.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@caleblloyd caleblloyd left a 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

deploy/crds.yml Outdated Show resolved Hide resolved
deploy/crds.yml Outdated Show resolved Hide resolved
Copy link
Member

@wallyqs wallyqs left a comment

Choose a reason for hiding this comment

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

LGTM

@caleblloyd caleblloyd merged commit 1464dd8 into nats-io:main Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants