-
Notifications
You must be signed in to change notification settings - Fork 2
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
Kafka version update #59
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just the one question, super slick!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems the kafka container doesn't have permission to create the /bitnami/kafka
directory and cannot start because of this.
Marking this as a draft for now to prevent it from being merged until the changes in #61 have been pushed through to USDOT dev. |
The changes in #61 have been merged into USDOT dev via usdot-jpo-ode#535, so we should be good to push changes to CDOT dev without it affecting the release. I am removing the 'draft' designation from this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to spin up the project successfully & the ODE was capable of consuming/producing to/from kafka, so that's good.
I did a quick search for zookeeper
and it looks like we've still got some references to Zookeeper in some files. These include the following:
- quickstart-compose.yml
- .devcontainer/kafka
- .devcontainer/post-create.sh
- docs/Architecture.md
- docs/dockerhub.md
- docs/Kubernetes.md
- docs/UserGuide.md
- docs/k8s-demo/values.yml
- docs/k8s-demo/templates/jpoode_kafka.yml
- docs/k8s-demo/templates/jpo_ode.yaml
- docs/k8s-demo/templates/jpo_zookeeper.yaml
- jpo-ode-consumer-example/README.md
These files should be looked at & updated/removed as necessary. If we don't want to use the new kafka version in the dev container, we should probably add a README to the .devcontainer
directory to make this clear.
The .devcontainer has been updated to run Kafka with Kraft and all .md files that pertain to running the ODE locally have been updated to remove any reference of Zookeeper. For the time being I've left the .yaml and Kubernetes.md files untouched as I wasn't sure if we'll want the Kubernetes demo to run Kafka with Kraft. Before migrating to Confluent we were still using Kafka with Zookeeper and so these yaml files will need to be updated if this is a change we'd like to make. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just the one comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is pretty darn cool
Updates the kafka version used by the standalone implementation of the ODE to the latest version of the Bitnami/kafka image (currently 3.6.1). Additionally, this update removes zookeeper and switches kafka over to use Kraft.
These changes have been tested locally using docker-compose. As no code changes were made existing unit tests are still passing.