Update GC cross-namespace note by liggitt · Pull Request #25091 · kubernetes/website

liggitt · 2020-11-17T22:39:08Z

Update note about cross-namespace references for garbage collection

xref kubernetes/kubernetes#92743

/cc @deads2k @jpbetz

k8sio-netlify-preview-bot · 2020-11-17T22:39:12Z

Deploy preview for kubernetes-io-vnext-staging processing.

Building with commit 8d96fcb

https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/5fb52c3c088a620007596158

liggitt · 2020-11-17T22:40:37Z

oops, pushed branch cut from master against dev-1.20... sorry for all the bogus language labels

krmayankk · 2020-11-18T07:39:57Z

+it is treated as having an unresolveable owner reference, and is not able to be garbage collected.
+
+In v1.20+, if the garbage collector detects an invalid cross-namespace
+or cluster-scoped -> namespaced `ownerReference`, it reports a warning Event 


nit: or cluster-scoped dependent referencing a namespaced ownerr or whatever is better than using the arrow ?

sftim

Hi, 3 tweaks I suggest - but don't let these block other reviewers from moving this PR forward. This will need a tech review.

liggitt · 2020-11-18T14:08:11Z

comments addressed.

sftim · 2020-11-18T14:21:23Z

I don't see an earlier technical LGTM, so I'm going to leave this pending that.
(if I missed one, happy to re-LGTM based on the changes since as they will be negligible)

krmayankk · 2020-11-18T23:33:30Z

nit: Generally i like to see a reason mentioned on why cross namespaces references should not be allowed , is it a performance issue or a security issue or something else. Might be good to put a one liner here or that is too much for the documentation, not sure.

/lgtm

k8s-ci-robot · 2020-11-18T23:33:44Z

LGTM label has been added.

Details

Git tree hash: 7a7519955d5a73d68be69cc292071f640c060d69

deads2k · 2020-11-19T13:41:05Z

This matches the PR and our intended behavior.

/lgtm

Generally i like to see a reason mentioned on why cross namespaces references should not be allowed

This goes back to the original design that keeps the GC design aligned to using namespaces as an ACL boundary. If we allow cross-namespace references, then it becomes possible for actions in one namespace to have impact in another. Several years ago, we were not ready to allow that interaction and today our position remains about the same. It's open for careful consideration, but we won't change the API to support it (requires a change to objectmeta) just because we could.

kbhawkey · 2020-11-19T15:55:53Z

/remove-language ja vi fr

kbhawkey · 2020-11-19T15:56:29Z

/remove-language es id zh

kbhawkey · 2020-11-19T15:57:25Z

/milestone 1.20

k8s-ci-robot · 2020-11-19T15:57:26Z

@kbhawkey: You must be a member of the kubernetes/website-milestone-maintainers GitHub team to set the milestone. If you believe you should be able to issue the /milestone command, please contact your Website milestone maintainers and have them propose you as an additional delegate for this responsibility.

Details

In response to this:

/milestone 1.20

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.

kbhawkey · 2020-11-19T15:58:47Z

-and owners that are cluster-scoped.
-2) Cluster-scoped dependents can only specify cluster-scoped owners, but not
-namespace-scoped owners.
+Cross-namespace owner references are disallowed by design.


nit: What is an "owner reference"? I am guessing the same as ownerReference?

ownerReferences are described in the paragraphs above

OK. It may help to break down the two cases (if I understand correctly). Does line 64 apply to the following paragraph or both pargraphs?
Is this true:
Dependent objects are only owned (referenced) by objects within their namespace or cluster scope.

There are two types of scoping that affect object ownership and garbage collection:

Namespaced
Namespaced owners must exist in the same namespace as the dependent.
...

Cluster-scoped
Cluster-scoped dependents can only specify cluster-scoped owners.
...

Hi @liggitt after this comment is addressed, I think this would be good to merge. PTAL

Hi. I think the text is okay to merge. In the future, the content could be updated -- make the text easier to digest and localize.
/lgtm

kbhawkey · 2020-11-19T16:02:51Z

Page preview:
https://deploy-preview-25091--kubernetes-io-vnext-staging.netlify.app/docs/concepts/workloads/controllers/garbage-collection/

krmayankk · 2020-11-19T21:44:34Z

This matches the PR and our intended behavior.

/lgtm

Generally i like to see a reason mentioned on why cross namespaces references should not be allowed

This goes back to the original design that keeps the GC design aligned to using namespaces as an ACL boundary. If we allow cross-namespace references, then it becomes possible for actions in one namespace to have impact in another. Several years ago, we were not ready to allow that interaction and today our position remains about the same. It's open for careful consideration, but we won't change the API to support it (requires a change to objectmeta) just because we could.

sorry i meant, is this something we should also document here ? I think from your and Jordan's answer, it has become clear and it makes sense to not allow, and putting that in the documentation will help our users.

kbhawkey · 2020-11-20T15:32:33Z

/cc @annajung

annajung · 2020-11-20T15:51:04Z

/milestone 1.20

annajung · 2020-11-20T15:51:12Z

/assign

sftim · 2020-11-23T16:44:10Z

/approve

k8s-ci-robot · 2020-11-23T16:44:27Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sftim

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~content/en/OWNERS~~ [sftim]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot requested review from deads2k and jpbetz November 17, 2020 22:39

liggitt force-pushed the gc branch from 6b3ba28 to d4ddf90 Compare November 17, 2020 22:39

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 17, 2020

liggitt changed the title Gc Update GC cross-namespace note Nov 17, 2020

liggitt mentioned this pull request Nov 17, 2020

Fix GC uid races and handling of conflicting ownerReferences kubernetes/kubernetes#92743

Merged

12 tasks

krmayankk reviewed Nov 18, 2020

View reviewed changes

Comment thread content/en/docs/concepts/workloads/controllers/garbage-collection.md

sftim reviewed Nov 18, 2020

View reviewed changes

Comment thread content/en/docs/concepts/workloads/controllers/garbage-collection.md Outdated

Comment thread content/en/docs/concepts/workloads/controllers/garbage-collection.md Outdated

Comment thread content/en/docs/concepts/workloads/controllers/garbage-collection.md Outdated

liggitt force-pushed the gc branch from d4ddf90 to 68544a0 Compare November 18, 2020 14:08

sftim reviewed Nov 18, 2020

View reviewed changes

Comment thread content/en/docs/concepts/workloads/controllers/garbage-collection.md Outdated

Update GC cross-namespace note

8d96fcb

liggitt force-pushed the gc branch from 68544a0 to 8d96fcb Compare November 18, 2020 14:14

k8s-ci-robot assigned krmayankk Nov 18, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 18, 2020

k8s-ci-robot assigned deads2k Nov 19, 2020

k8s-ci-robot removed language/ja Issues or PRs related to Japanese language language/vi Issues or PRs related to Vietnamese language language/fr Issues or PRs related to French language labels Nov 19, 2020

k8s-ci-robot removed language/es Issues or PRs related to Spanish language language/id Issues or PRs related to Indonesian language language/zh Issues or PRs related to Chinese language labels Nov 19, 2020

kbhawkey reviewed Nov 19, 2020

View reviewed changes

k8s-ci-robot requested a review from annajung November 20, 2020 15:32

k8s-ci-robot added this to the 1.20 milestone Nov 20, 2020

k8s-ci-robot assigned annajung and kbhawkey Nov 20, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 23, 2020

k8s-ci-robot merged commit ffd2d2e into kubernetes:dev-1.20 Nov 23, 2020

liggitt deleted the gc branch January 14, 2021 20:01

Conversation

liggitt commented Nov 17, 2020

Uh oh!

k8sio-netlify-preview-bot commented Nov 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

liggitt commented Nov 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

krmayankk Nov 18, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sftim left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

liggitt commented Nov 18, 2020

Uh oh!

Uh oh!

sftim commented Nov 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

krmayankk commented Nov 18, 2020

Uh oh!

k8s-ci-robot commented Nov 18, 2020

Uh oh!

deads2k commented Nov 19, 2020

Uh oh!

kbhawkey commented Nov 19, 2020

Uh oh!

kbhawkey commented Nov 19, 2020

Uh oh!

kbhawkey commented Nov 19, 2020

Uh oh!

k8s-ci-robot commented Nov 19, 2020

Uh oh!

kbhawkey Nov 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liggitt Nov 19, 2020

Choose a reason for hiding this comment

Uh oh!

kbhawkey Nov 19, 2020

Choose a reason for hiding this comment

Uh oh!

annajung Nov 23, 2020

Choose a reason for hiding this comment

Uh oh!

kbhawkey Nov 23, 2020

Choose a reason for hiding this comment

Uh oh!

kbhawkey commented Nov 19, 2020

Uh oh!

krmayankk commented Nov 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kbhawkey commented Nov 20, 2020

Uh oh!

annajung commented Nov 20, 2020

Uh oh!

annajung commented Nov 20, 2020

Uh oh!

sftim commented Nov 23, 2020

Uh oh!

k8s-ci-robot commented Nov 23, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

k8sio-netlify-preview-bot commented Nov 17, 2020 •

edited

Loading

liggitt commented Nov 17, 2020 •

edited

Loading

sftim commented Nov 18, 2020 •

edited

Loading

kbhawkey Nov 19, 2020 •

edited

Loading

krmayankk commented Nov 19, 2020 •

edited

Loading