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

[bitnami/kafka] Adding option to gracefully shutdown sidecar container in provisioning pod #5835

Closed
nc-ruth opened this issue Mar 18, 2021 · 5 comments · Fixed by #5890
Closed

Comments

@nc-ruth
Copy link
Contributor

nc-ruth commented Mar 18, 2021

Which chart:
Bitnami Kafka 12.11.1

Is your feature request related to a problem? Please describe.
My cluster is using Istio for traffic control/shaping, and Kafka works really well with Istio.
I have an issue with the provisioning job, the provisioning job is not marked as completed due to the sidecar container staying in a running state.

See more about the issue here:
istio/istio#11659
istio/istio#15041
istio/istio#6324

Describe the solution you'd like
I see two different approaches that might solve the issue:

Allow the provisioning job to run without the Istio sidecar.
With the use of podAnnotations to the pod generated by the kafka-provisioning job, and adding the annotation:
sidecar.istio.io/inject: "false" (See example here: istio/istio#11659 (comment) )

The disadvantage with this approach is:

  • Istio will not be used for communication between the kafka topic provisioning pod and the kafka brokers
  • Istio's mutual TLS (mTLS) feature will not work between Kafka topic provisioning pod and the kafka brokers.
  • Istio can not be run in a strict mTLS mode.

The advantage with this approach is:

  • Easiest "solution" to implement. Adding the possibility to set podAnnotations to the generated pod by the job. (See example here for the kafka-provisioning job: nc-ruth@2da7bf2 )

Allow the provisioning job to run code after finished provisioning of topics.
I don't know how to implement this, the most efficient way?

Describe alternatives you've considered
I have exec'ed into the sidecar container, and curl'ed to quitquitquit endpoint, which afterwards shutdowns the sidecar container and completes the job successfully.

Additional context
Add any other context or screenshots about the feature request here.
kubectl get pods
Kafka-cluster-with-provisioning

kubectl logs <topic-provisioning-pod> # just finished
Topic provisioning succeeded

kubectl get pods # after the provisioning job is done
status-state-not-ready

kubectl describe pod <topic-provisioning-pod> # just finished provisioning topics - shows the provisioning container state
provisioning-job-succeded

kubectl describe pod <topic-provisioning-pod> # just finished provisioning topics - shows the Istio sidecar container state
istio-proxy-running

kubectl exec -it <topic-provisioning-pod> -c istio-proxy -- curl -fsI -X POST http://localhost:15020/quitquitquit
exit-the-sidecar-gracefully

kubectl get pods # after sidecar has been terminated
pod-status-after-terminated

@juan131
Copy link
Contributor

juan131 commented Mar 18, 2021

Hi @nc-ruth

Thanks for bringing up this topic!

First of all, our charts are not designed to work with Istio. That said, they can be integrated with Istio since they can be extended adding custom sidecar containers, labels, annotations, etc.
I want to be clear on this because we don't test the integration with Istio in our pipeline and, therefore, we don't officially support it. As a consequence, we cannot guarantee that we will not break the compatibility with Istio in the future since we're not taking into account during the development process any possible consequence affecting this integration.


Sticking to the topic you brought up, please correct me if I'm wrong but basically your goal is to mark the provisioning job as "completed" regardless the state of the sidecar container that Istio injects in every pod, right?

Allow the provisioning job to run code after finished provisioning of topics.

What is the purpose of this? Doing some "curl" call that shutdowns the Istio sidecar? I guess we could implement some provisioning.command/provisioning.args parameters that allow you to overwrite the default ones, so you can add any piece of code you want. Would that work?

@nc-ruth
Copy link
Contributor Author

nc-ruth commented Mar 22, 2021

Hi @juan131,

Thank you very much for your answer!

I completely understand that your charts are not designed to work with Istio, and it's a bonus if it works with Istio.
It was not the intention of this issue, to "hardcode" in the istio-sidecar annotation into the chart. I apologize, if you understood the issue as that.

I make sense that your pipelines does not test the Kafka chart with Istio integration, and it makes no sense to do so. I mean, there is a lot of integrations, and if your pipeline had to test each one out (and in combination), it would be a nightmare.

I understand that you can't guarantee the compability with Istio in the future.


Back to the topic, the solution you are bringing up will solve our issue (the usecase). This is actually the solution I prefer. Is there any other charts, that have some similar implementation with provisioning.command / provisioning.args?

How would it work, if it is only extending the already supported and tested command (within your pipeline)?

@juan131
Copy link
Contributor

juan131 commented Mar 23, 2021

Hi @nc-ruth

Thanks for your consideration.

Regarding the issue, please take a look to the PR I just created. Using these changes, you should be able to set in your values.yaml any custom command or args you want to use in the provisioning container.

@nc-ruth
Copy link
Contributor Author

nc-ruth commented Mar 24, 2021

Hi @juan131,

Thank you for the PR!

I have pulled down your PR, and tested the base case (without any args or commands specified) and it works well.
I have tested the PR with specifiying provisioning.args and that works as intended.

What is needed for this PR to be accepted and merged into master?

Once again, thank you for the PR and answers :)


values.yaml file that uses provisioning.args

istioEnabled: true
provisioning:
  enabled: true
  args:
  - -ec
  - |
    {{- $bootstrapServer := printf "%s:%d" (include "kafka.fullname" .) (.Values.service.port | int64) }}
    {{- range $topic := .Values.provisioning.topics }}
    echo "Ensure topic '{{ $topic.name }}' exists"
    /opt/bitnami/kafka/bin/kafka-topics.sh \
      --create \
      --if-not-exists \
      --bootstrap-server {{ $bootstrapServer }} \
      --replication-factor {{ $topic.replicationFactor }} \
      --partitions {{ $topic.partitions }} \
      {{- range $name, $value := $topic.config }}
      --config {{ $name }}={{ $value }} \
      {{- end }}
      --topic {{ $topic.name }}
    {{- end }}
    {{- if (eq .Values.istioEnabled true) }}
    curl -fsI -X POST http://localhost:15020/quitquitquit
    {{- end }}
    echo "Provisioning succeeded"

@juan131
Copy link
Contributor

juan131 commented Mar 24, 2021

Hi @nc-ruth

The PR was just merged! Thanks for your feedback and helping us to improve the chart.

Thanks also for sharing the values.yaml you used to manage the Istio integration!

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.

2 participants