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

Add "OwnerType" Annotation for Non-GC'ed DependentResources #2585

Open
coltmcnealy-lh opened this issue Nov 16, 2024 · 5 comments · Fixed by #2606
Open

Add "OwnerType" Annotation for Non-GC'ed DependentResources #2585

coltmcnealy-lh opened this issue Nov 16, 2024 · 5 comments · Fixed by #2606

Comments

@coltmcnealy-lh
Copy link
Contributor

I have a reconciler that extends KubernetesCRUDNoGCDependentResource. The created dependent has the following annotations:

metadata:
  annotations:
    io.javaoperatorsdk/primary-name: my-thing
    io.javaoperatorsdk/primary-namespace: my-namespace

In my operator, I have several different CustomResourceDefinitions and reconcilers. Therefore, I can and do have multiple custom resources in the my-namespace namespace called my-thing.

In my own code, I need to determine the owner of these resources. I can solve that by putting labels/annotations on them myself (this is quite a bit of work). However, I imagine that this is likely a vulnerability to bugs.

Proposed Solution

Add another annotation to NoGC resources and use that for determining ownership:

metadata:
  annotations:
    io.javaoperatorsdk/primary-type: MyCustomResource
@metacosm
Copy link
Collaborator

I need to double check but isn't the type of the primary known when the annotations are used?

@metacosm
Copy link
Collaborator

Indeed, the owner type is not known. However, this will require deeper changes than simply adding the annotation because the mappers only return ResourceID instances, which, currently, only also record name and namespaces.

@csviri
Copy link
Collaborator

csviri commented Nov 16, 2024

@metacosm I'm pretty sure we don't need to extend resourceID, note that from v5 we do type checking for owner references, adding type annotation should do the trick, and have the secondary filter on the.

Another option is just use labels/ label selectors, which is also more efficient.

or rather both. But something like the primary type should be there fo sure - preferably a GroupVersionKind (GVK) as a value in some form.

@csviri csviri linked a pull request Nov 17, 2024 that will close this issue
@csviri
Copy link
Collaborator

csviri commented Nov 17, 2024

#2586

added scretch how the impl could look like, will finalize it bit later. @coltmcnealy-lh should we backport it into 4.x or you are ok to wait for v5?

@coltmcnealy-lh
Copy link
Contributor Author

coltmcnealy-lh commented Nov 17, 2024

@csviri we have a workaround, which is just overriding a bunch of getSecondaryResources methods to use the KubernetesClient directly and filter based on labels that we manually put onto our dependents. We can wait until v5 (we do our best to stay up-to-date with the JOSDK). Just thought I'd report this in case someone else is affected.

Thanks for the prompt response!

@csviri csviri linked a pull request Nov 22, 2024 that will close this issue
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 a pull request may close this issue.

3 participants