3scale-ops / prometheus-exporter-operator

Operator to centralize the setup of 3rd party prometheus exporters on Kubernetes/OpenShift, with a collection of grafana dashboards
Apache License 2.0
42 stars 15 forks source link

Enhance: Refactoring of structure #15

Closed victorock closed 3 years ago

victorock commented 3 years ago

Better accommodate exporters.

raelga commented 3 years ago

Hello @victorock, thanks for reaching out!

The current structure is tied to the way the OperatorSDK works, you can see the generated scaffold here: https://sdk.operatorframework.io/docs/building-operators/ansible/tutorial/

Can you provide more information regarding your proposal?

victorock commented 3 years ago

@raelga sorry about not clarifying what i meant. I was referring to the ansible content under: roles/prometheusexporter/.

raelga commented 3 years ago

What did you have in mind?

victorock commented 3 years ago

@raelga follow here an example to validate the idea, before going through testing, etc...

I've also integrated some additional content and logic to be able to better handle the lifecycle of the operator itself:

With this implementation, tasks moved to here, so we avoid accidental changes by users integrating new exporters...

As such, users only care about changing exporters specifications here...

slopezz commented 3 years ago

Thanks for the input @victorock, let us review it carefully as there are many changes, and we will get back to you, but the overall looks good!

raelga commented 3 years ago

@victorock your changes look really good, thanks for the improvements.

I can't think right now, for the current operator, an escenario that pre/post task may come helpful who knows. The rest of the changes, makes the code more clean and extensible.

Feel free to push a pull request so we can do a final review and integrate your changes. :)

slopezz commented 3 years ago

@victorock, as @raelga said, the overall looks really nice, cleaner and easier to integrate a new exporter. Just a few comments/suggestions before opening the pull request:

slopezz commented 3 years ago

Closing issue, it was resolved on https://github.com/3scale-ops/prometheus-exporter-operator/pull/24 where we applied some operator improvements.