druid-io / druid-operator

Druid Kubernetes Operator
Other
205 stars 93 forks source link

Proposal: Add sidecar to Druid deployments #293

Open cintoSunny opened 2 years ago

cintoSunny commented 2 years ago

MOTIVATION

Currently, the Druid operator does not allow the addition of a sidecar container to Druid deployments. The only way is to inject sidecars by mutation webhooks like istio. But many orgs may not allow setting up istio or it could be an additional effort to add them.

PROPOSAL To have the ability to add sidecars via druid operator based on a set of configs. The configs can look something like this:

sidecarContainer:
- configs:
    imageName: docker.com/ns/samplesimage:latest
    containerName: sidecar
    command: /bin/start.sh
    pullPolicy: Always
...

I have a working code for my org that I used for logging. And can push the change here, but wanted to check with the community first. I saw a couple of issues regarding this earlier, so thought it would be useful. cc: @AdheipSingh

AdheipSingh commented 2 years ago

IMHO mutation webhook is the right way to inject, security reasons are something specific to orgs. Is there a druid specific reason to add a sidecar ? Regardless i would be curios to see the code changes as this might require changes in the core handler and makeDeployment and makeStatefulset functions.

AdheipSingh commented 2 years ago

cc @nishantmonu51

cintoSunny commented 2 years ago

Thanks. I agree that mutation webhook may be a better solution, but may not be an option for everyone. I believe the operator should have easy config-driven settings to enable sidecars as well. I agree with @himanshug here: https://github.com/druid-io/druid-operator/issues/59#issuecomment-729279487

I see a couple of issues open related to this as well. These were also my motivation to enable sidecars: https://github.com/druid-io/druid-operator/issues/59 https://github.com/druid-io/druid-operator/issues/275

For us, we had to send logs to Splunk which needed a sidecar to be run.

I will send the PR across in a while.

AdheipSingh commented 2 years ago

Init containers make sense to pull in dependency , adding sidecars in the CR doesn't make sense to me. Sidecars are something which are injected for external use cases such as logging, security. I would only add a sidecar config inside the CR if its a use case for druid specifically.

Mutation Webhooks is the standard practice to add sidecars.

You can add mutation webhook in the source code, ie is the standard practice as per kubebuilder and controller runtime docs. Adding in the CR will add in anti-practice and confusion

cintoSunny commented 2 years ago

I get that Mutation Webhooks is the standard practice, but thought it will be good to have an option for the sidecar in the config itself. Let me know if you still want to take a look at the PR. I can send it up within a few days.

AdheipSingh commented 2 years ago

@cintoSunny feel free to send PR :)

cintoSunny commented 2 years ago

@AdheipSingh : Here is the PR: https://github.com/druid-io/druid-operator/pull/296