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

Issues related to Child Resources having no owner references #488

Closed
neowulf opened this issue Mar 4, 2024 · 3 comments · Fixed by #493
Closed

Issues related to Child Resources having no owner references #488

neowulf opened this issue Mar 4, 2024 · 3 comments · Fixed by #493
Labels
bug Something isn't working

Comments

@neowulf
Copy link

neowulf commented Mar 4, 2024

Describe the bug

When creating children in a namespace different than the resource's namespace - the owner references can't be used. In such cases, OurChild func needs to be implemented. Under these circumstances,

  1. the tracked children get created more than once when the resource is created / updated.
  2. the tracked children are not deleted when the resource is deleted.

Reproduction steps

.

Expected behavior

.

Additional context

No response

@neowulf neowulf added the bug Something isn't working label Mar 4, 2024
@neowulf neowulf changed the title Child Resources having no owner references are created more than once Issues related to Child Resources having no owner references Mar 4, 2024
@nebhale
Copy link
Member

nebhale commented Mar 5, 2024

I believe you're supposed to implement the ListOptions function here.

@neowulf neowulf closed this as not planned Won't fix, can't repro, duplicate, stale Mar 5, 2024
@scothis
Copy link
Contributor

scothis commented Mar 6, 2024

Yea, the intent is to implement ListOptions to override the default value. I do think there's something we can do to make this case more discoverable rather than being surprised by resources being created en masse and not cleaned up. A few possible approaches:

  1. change the default ListOptions when SkipOwnerReference is true (what you proposed in Child Resources having no owner references are created more than once #489)
  2. require the user to specify ListOptions when SkipOwnerReference is true
  3. deprecate ListOptions

Assuming there is an informer defined for the child resource, using ListOptions isn't much faster than the equivalent OurChild resource. It does make a difference if the List client request actually hits the API server.

Option 2 feels like a good middle ground. It will be a breaking change for users who only operate in the same namespace but don't want to use owner refs (I'm not sure why someone would need that). The reconciler will fail fast at startup rather than spam the API server with orphaned resources.

What do you think?

@mamachanko
Copy link
Collaborator

  1. change the default ListOptions when SkipOwnerReference is true (what you proposed in Child Resources having no owner references are created more than once #489)
  2. require the user to specify ListOptions when SkipOwnerReference is true
  3. deprecate ListOptions

[...]

Option 2 feels like a good middle ground. It will be a breaking change for users who only operate in the same namespace but don't want to use owner refs (I'm not sure why someone would need that). The reconciler will fail fast at startup rather than spam the API server with orphaned resources.

What do you think?

➕→2️⃣

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
4 participants