HNC: consider simplifying the inheritance of allowCascadingDelete

[adrianludwin]

Right now, allowCascadingDelete is only inherited up through subnamespaces to the first full namespace. This is probably the "right" thing to do but it's kind of hard to explain. It might be simpler to just accept it if any ancestor has it, which is actually what we say in the user guide right now (the real definition is just shown in a "note" block).

/assign @yiqigao217

Yiqi, wdyt?

/good-first-issue

[k8s-ci-robot]

@adrianludwin:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

Right now, allowCascadingDelete is only inherited up through subnamespaces to the first full namespace. This is probably the "right" thing to do but it's kind of hard to explain. It might be simpler to just accept it if any ancestor has it, which is actually what we say in the user guide right now (the real definition is just shown in a "note" block).

/assign @yiqigao217

Yiqi, wdyt?

/good-first-issue

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[yiqigao217]

My concern is that users may set it for a one-time deletion and forget to unset it. To support this simplified inheritance, we may need to come up with a new mechanism to prevent that case? WDYT @adrianludwin

[adrianludwin]

I agree it's a bit less safe - but not *that* much less safe. You should already be setting it on the lowest possible level anyway, and usually you won't have full and subnamespaces interleaved either. So the way it works now just seems a bit arbitrary. The only other option I can think of is to get rid of the inheritance completely. You can only delete is aCD is set on the *current *namespace, or if your immediate parent is *already *being deleted.

…

On Fri, May 15, 2020 at 2:42 PM yiqigao217 ***@***.***> wrote: My concern is that users may set it for a one-time deletion and forget to unset it. To support this simplified inheritance, we may need to come up with a new mechanism to prevent that case? WDYT @adrianludwin <https://github.com/adrianludwin> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#730 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AE43PZB4HACQ2AFGBPB6ABLRRWEJ7ANCNFSM4NBSDI4Q> .

[rcolline]

What is the usecase justifying cascading deletes? Do we expect users to often want to delete subtrees? I feel like namespace trees will not be something that have short lifecycle, this is not a filesystem.

I guess I am questioning the overall product motivation for such a feature. The downsides is that it creates a SPOF with a large blast radius.

[yiqigao217]

Hi @rcolline , this is a great question. Cascading delete only applies to subnamespaces that are created by HNC from subnamespaceAnchor CRs in parent (regular) namespaces for users without cluster-level privileges to create namespaces. Regular namespaces will never be cascading deleted.

Subnamespaces are created/deleted by creating/deleting the anchor CR in the parent. Therefore, cascading delete would happen on subnamespace chains. That's also why we have allowCascadingDelete to prevent the deletion by accident or not being aware of existing subnamespaces.

Please let me know if it doesn't answer your question or if you were asking why we need cascading delete on subnamespaces.