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

Reconcile not called if the CR is Cluster scoped and the owns children are namespaced #293

Closed
praveenperera opened this issue Jul 24, 2020 · 7 comments · Fixed by #294
Closed
Assignees
Labels
bug Something isn't working runtime controller runtime related

Comments

@praveenperera
Copy link
Contributor

praveenperera commented Jul 24, 2020

Example: My custom resource CertIssuer is not scoped to a namespace. The CRD for is scope: Cluster.

I want my controller to be able to manage certificates across the cluster. Across many different namespaces.

Its children are Secrets. But Secrets have to be scoped to a namespace.

In this case the children (Secrets) would never trigger the reconcile function. Right?

Maybe a solution is if the selector field was public. I could make my own version of owns using trigger_with.

Or for a situation like this should I just be using watchers directly instead of using controller?

@praveenperera praveenperera changed the title owns does not trigger_owners if the CR is Cluster scoped and the children are namespace owns does not trigger_owners if the CR is Cluster scoped and the children are namespace objects Jul 24, 2020
@praveenperera praveenperera changed the title owns does not trigger_owners if the CR is Cluster scoped and the children are namespace objects Reconcile not called if the CR is Cluster scoped and the children are namespaced Jul 25, 2020
@praveenperera praveenperera changed the title Reconcile not called if the CR is Cluster scoped and the children are namespaced Reconcile not called if the CR is Cluster scoped and the owns children are namespaced Jul 25, 2020
@clux
Copy link
Member

clux commented Jul 25, 2020

if you've hooked up:

let certs = Api<CertIssuer>::all();
let secrets = Api<Secret>::all();
Controller::new(certs, lp).owns(secrets, lp)

then reconcile should get triggered on all secrets (maybe you wanna scope secret listparams to some selector - to limit the amount of secret events this will yield).
If you said Api::namespaced(ns) for Api<Secret> then you're bound to only one namespace.

@clux clux added invalid rejected as a valid issue runtime controller runtime related labels Jul 25, 2020
@praveenperera
Copy link
Contributor Author

praveenperera commented Jul 25, 2020

Hey I think I could have explained this better.

This is exactly how I have it set up. However, even this way, the reconcile is never triggered if I change any of the secrets.

let certs = Api<CertIssuer>::all();
let secrets = Api<Secret>::all();
Controller::new(certs, lp).owns(secrets, lp)

I think I know the reason why.

My CertIssuer is a Cluster scoped custom resource, so it doesn't have a namespace in metadata. It looks like this:

apiVersion: certmaster.kuberails.com/v1
kind: CertIssuer
metadata:
  name: com-ok-kuberails
spec:
  domainName: praveenperera.com
  namespaces:
    - default
    - praveen
  dnsProvider:
    provider: cloudflare
    key: thisIsMyKey
    secretKey: thisIsMySecretKey

However all secrets are namespaced.

https://github.com/clux/kube-rs/blob/master/kube-runtime/src/controller.rs#L85-L92

Here it looks like it uses the namespace of the owner to find the children? But the namespace of the owner is always None and all the children with have Some(namespace)

@clux
Copy link
Member

clux commented Jul 25, 2020

Oh, right. Own requires you to put owner references on the secret object, otherwise the controller had no way to connect it to the right crd. Sounds like you are missing that?

@praveenperera
Copy link
Contributor Author

praveenperera commented Jul 25, 2020

I actually do setup the owner references following the example.

I made the repo I was working on public. You can see it in the setup-watches branch:

https://github.com/kuberails/certmaster/blob/setup-watches/controller/src/main.rs#L155-L173

And here is where I setup the controller:

https://github.com/kuberails/certmaster/blob/setup-watches/controller/src/main.rs#L109-L123

@nightkr
Copy link
Member

nightkr commented Jul 25, 2020

Oh right, I see the problem now.

@nightkr nightkr self-assigned this Jul 25, 2020
nightkr added a commit to nightkr/kube-rs that referenced this issue Jul 25, 2020
Fixes kube-rs#293.

This is a bit magical, but it's fairly common in K8s to allow reference
fields (such as `OwnerReferences`) to refer to either cluster-scoped objects
or namespaced resources in the same namespace.

Code that converts those events into `ObjectRef`s generally won't know which of
these cases is applicable, so this is the best we can do.
@clux clux added bug Something isn't working and removed invalid rejected as a valid issue labels Jul 25, 2020
@clux clux closed this as completed in #294 Jul 26, 2020
@clux
Copy link
Member

clux commented Jul 26, 2020

Does this work better for you @praveenperera ? If there's a need for it, I can probably push a patch for this tomorrow.

@praveenperera
Copy link
Contributor Author

praveenperera commented Jul 26, 2020

Yes I think this will be perfect. Thanks guys. I am going to try it out tomorrow.

I'll use master, no rush to release a new version from my side.

Edit

Just tried it out, and it works perfectly now. Thanks!

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

Successfully merging a pull request may close this issue.

3 participants