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

Watching all v1.Secrets in parallel of watching CRD #694

Closed
prune998 opened this issue Nov 2, 2018 · 12 comments
Closed

Watching all v1.Secrets in parallel of watching CRD #694

prune998 opened this issue Nov 2, 2018 · 12 comments
Assignees

Comments

@prune998
Copy link

prune998 commented Nov 2, 2018

Type of question

how to implement a specific feature

Question

I'm building an operator which, depending on the CR definition, will merge some TLS Secrets into another Opaque Secret, one file per merged secret.

I'v done the part of creating the CRD, watching for it and for Operator created secrets.

Every time a secret is created/updated, I also need to find out if it matches one of the CR search definition (by labels), and if it does, trigger the re-creation (by enqueueing the CR to the Reconcile)

I'm stuck with the last part.
Should I create a new Controler to watch for v1.Secret ?
Should I add a EnqueueRequestForObject in my CRD controler ?

Maybe there's a irc/forum/slack where I could get some help ?
Thanks.

Environment

  • operator-sdk version: v0.1.0+git

  • Kubernetes version information:

Client Version: version.Info{Major:"1", Minor:"11", GitVersion:"v1.11.0", GitCommit:"91e7b4fd31fcd3d5f436da26c980becec37ceefe", GitTreeState:"clean", BuildDate:"2018-06-27T22:30:22Z", GoVersion:"go1.10.3", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"10+", GitVersion:"v1.10.7-gke.6", GitCommit:"06898a4d0f2b96f82b43d9e59fa2825bd3d616a2", GitTreeState:"clean", BuildDate:"2018-10-02T17:32:01Z", GoVersion:"go1.9.3b4", Compiler:"gc", Platform:"linux/amd64"}
  • Kubernetes cluster kind: GKE
@joelanford
Copy link
Member

@prune998 If I'm following, it sounds like you can use just one controller that has two watches:

  • EnqueueRequestForObject to watch for your CRD resource changes. In your CRD reconciler, you'd fetch all of the TLS secrets using a label selector based on the search definition and then run your merge logic with the matched secrets.
  • EnqueueRequestFromMapFunc to watch for secret changes and trigger a reconcile of one or more CRs. In your mapper function, you'd fetch all of the CRs. For each CR that has a search definition that matches the passed in secret, you'd create a new reconcile.Request for the CR, and return the list of requests, which would trigger your CRD reconciler for each CR that matched.

If you aren't already, you may also want to use owner references on the opaque merged secrets and another watch with EnqueueRequestForOwner in your CRD controller. That gives you:

  • Automatic garbage collection of the opaque secrets when the CR is deleted.
  • An opportunity to reconcile your CR if its underlying opaque secret changes.

@prune998
Copy link
Author

prune998 commented Nov 5, 2018

Thanks @joelanford for this help.
In the meantime I went with a second controler. I'll check for EnqueueRequestFromMapFunc as I never saw it before.

I'm already using EnqueueRequestForOwner for the secrets created by my Operator, from the CR description. But I can't use it for other secrets which are not created by me.

I'm now stuck where the controler who's watching for Secret creation need to trigger a reconcile on my CR (enqueue) ...
I thought I could just update the CR and change it's status ?

@prune998
Copy link
Author

prune998 commented Nov 5, 2018

@joelanford, I just tested EnqueueRequestFromMapFunc. I think this allows me to do what I want, BUT...
Maybe i'm misunderstanding it, but following the example at https://godoc.org/github.com/kubernetes-sigs/controller-runtime/pkg/handler#example-EnqueueRequestsFromMapFunc, I do get the Reconcile function triggered on Secret modification, but I only get a types.NamespacedName, which does not allows me to check the type of the "event".
How would I check if it's a CR modification or a Secret modification ?

@joelanford
Copy link
Member

I'm already using EnqueueRequestForOwner for the secrets created by my Operator, from the CR description. But I can't use it for other secrets which are not created by me.

Correct, EnqueueRequestForOwner would only be for the Secrets created by the operator for CRs.

How would I check if it's a CR modification or a Secret modification ?

I don't think you would need to check. In the mapper function for EnqueueRequestFromMapFunc, you're implementing the logic that says "when Secret A is created/updated, reconcile all of the CRs that have a search criteria that match Secret A." In your reconcile function, you would reconcile your CR just like you would if the EnqueueRequestForObject watch triggered due to a created/updated CR. Either way, you're getting a list of Secrets, filtering based on the CR search criteria, and merging them together.

@joelanford joelanford self-assigned this Nov 5, 2018
@prune998
Copy link
Author

prune998 commented Nov 5, 2018

Sounds you gave me the solution @joelanford.
I found this which kind of do the same thing and went the same way.

It's now fully working.

My concern now is that the Reconcile seem to be triggered more than once for the same API change..
Ex removing the default/false-test-cert-operator-by-labels secret :

{"level":"info","msg":"Secret default/false-test-cert-operator-by-labels triggered in CertMerge","time":"2018-11-05T14:16:27Z"}
{"level":"info","msg":"CertMerge default/test-certmerge-labels added to Reconcile","time":"2018-11-05T14:16:27Z"}
{"level":"info","msg":"Secret default/false-test-cert-operator-by-labels triggered in CertMerge","time":"2018-11-05T14:16:27Z"}
{"level":"info","msg":"CertMerge default/test-certmerge-labels added to Reconcile","time":"2018-11-05T14:16:27Z"}
{"level":"info","msg":"Reconciling CertMerge default/test-certmerge-labels","time":"2018-11-05T14:16:27Z"}
{"certmerge":"test-certmerge-labels","level":"info","msg":"adding cert default/test-ingressgateway-certs to default/test-cert-labels","namespace":"default","time":"2018-11-05T14:16:27Z"}
{"level":"info","msg":"Updating Secret default/test-cert-labels\n","time":"2018-11-05T14:16:27Z"}
{"level":"info","msg":"Secret default/false-test-cert-operator-by-labels triggered in CertMerge","time":"2018-11-05T14:16:27Z"}
{"level":"error","msg":"Unable to retrieve Secret default/false-test-cert-operator-by-labels from store: Secret \"false-test-cert-operator-by-labels\" not found","time":"2018-11-05T14:16:27Z"}

As you can see the removal of secret default/false-test-cert-operator-by-labels is triggered 3 times until it's definitely not there anymore.
Maybe I should wait a little bit the first time until the API settle ?

@joelanford
Copy link
Member

Is this issue only happening on deletes? On a delete, objects may get updated multiple times by Kubernetes and other controllers (e.g. setting DeletionTimestamp, removing finalizers, etc.), so you may need to add extra logic to your map function to skip reconciliation of the CR in certain situations.

The other thing that you may notice is that when the merged secrets are created or updated by the operator, I'm pretty sure your map function will be triggered (since its watching all secrets). So you'll probably want to ignore those in the map function since they'll be handled by the EnqueueRequestForOwner watch.

@prune998
Copy link
Author

prune998 commented Nov 5, 2018

Yep.
Many thanks for all your help @joelanford !
I'm polishing my operator and release it soon.

@joelanford
Copy link
Member

No problem! Glad I could help.

@prune998
Copy link
Author

prune998 commented Nov 6, 2018

@joelanford if you get this, maybe you could help me again...

I'm trying to use predicate to filter the events.
The event.UpdateEvent is composed of :

MetaOld v1.Object
ObjectOld runtime.Object
MetaNew v1.Object
ObjectNew runtime.Object

I would like to compare the Secrets Data (name and base64 value) of the old and new to see if something change.
This way I'll be able to filter the real secret change versus the metata-data change that should not trigger a reconcile.

I can't find a way to access the data inside the runtime.Object
(I'm sure this is a noob question but my Go skills (and my K8s API Go-client skills) are quite... low)...

Thanks !

@joelanford
Copy link
Member

@prune998 you can use a type assertion to get the runtime.Object's underlying value. If you setup the watch that uses this predicate with &source.Kind{Type: &v1.Secret{}}, the underlying values of ObjectOld and ObjectNew will be of type *v1.Secret.

@prune998
Copy link
Author

prune998 commented Nov 6, 2018

Well, I was trying to use type assertion without success when I reached to you but I finaly got it working !
Thanks again for your time !

@prune998
Copy link
Author

prune998 commented Nov 7, 2018

I've released my operator and blogged about it. Comments welcome :)
https://medium.com/@prune998/istio-1-0-2-envoy-cert-manager-lets-encrypt-for-tls-certificate-merge-7a774bff66c2
https://github.com/prune998/certmerge-operator

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

No branches or pull requests

2 participants