-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
feat(dev): Use newer kafka & zookeeper images to support Apple arm64 #29084
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -158,12 +158,16 @@ runs: | |
|
||
# TODO: Use devservices kafka. See https://github.com/getsentry/sentry/pull/20986#issuecomment-704510570 | ||
if [ "$NEED_KAFKA" = "true" ]; then | ||
# This is *not* the production version. Unclear reason as to why this was chosen | ||
# https://github.com/getsentry/ops/blob/c823e62f930ecc6c97bb08898c71e49edc7232f6/cookbooks/getsentry/attributes/default.rb#L631 | ||
docker run \ | ||
--name sentry_zookeeper \ | ||
-d --network host \ | ||
-e ZOOKEEPER_CLIENT_PORT=2181 \ | ||
confluentinc/cp-zookeeper:4.1.0 | ||
|
||
# This is the production version; do not change w/o changing it there as well | ||
# https://github.com/getsentry/ops/blob/c823e62f930ecc6c97bb08898c71e49edc7232f6/cookbooks/getsentry/attributes/default.rb#L643 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just taking note of it. Production uses |
||
docker run \ | ||
--name sentry_kafka \ | ||
-d --network host \ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ | |
import sys | ||
import tempfile | ||
from datetime import timedelta | ||
from platform import platform | ||
from urllib.parse import urlparse | ||
|
||
from django.conf.global_settings import * # NOQA | ||
|
@@ -1700,6 +1701,8 @@ def build_cdc_postgres_init_db_volume(settings): | |
) | ||
|
||
|
||
APPLE_ARM64 = platform().startswith("mac") and platform().endswith("arm64-arm-64bit") | ||
|
||
SENTRY_DEVSERVICES = { | ||
"redis": lambda settings, options: ( | ||
{ | ||
|
@@ -1752,15 +1755,24 @@ def build_cdc_postgres_init_db_volume(settings): | |
), | ||
"zookeeper": lambda settings, options: ( | ||
{ | ||
"image": "confluentinc/cp-zookeeper:5.1.2", | ||
# On Apple arm64, we upgrade to version 6.x allows zookeeper to run properly on Apple's arm64 | ||
# See details https://github.com/confluentinc/kafka-images/issues/80#issuecomment-855511438 | ||
# I'm selectively upgrading the version on Apple's arm64 since there was a bug that only affects | ||
# Intel machines. For more details see: https://github.com/getsentry/sentry/pull/28672 | ||
"image": "confluentinc/cp-zookeeper:6.2.0" | ||
if APPLE_ARM64 | ||
else "confluentinc/cp-zookeeper:5.1.2", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm selectively upgrading the version on Apple's arm64 since there was a bug that only affects Intel (You can read more details in #28672). I've added a card to investigate this later on. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would add this explanation in the comments There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added |
||
"environment": {"ZOOKEEPER_CLIENT_PORT": "2181"}, | ||
"volumes": {"zookeeper": {"bind": "/var/lib/zookeeper"}}, | ||
"only_if": "kafka" in settings.SENTRY_EVENTSTREAM or settings.SENTRY_USE_RELAY, | ||
} | ||
), | ||
"kafka": lambda settings, options: ( | ||
{ | ||
"image": "confluentinc/cp-kafka:5.1.2", | ||
# On Apple arm64, we upgrade to version 6.x to match zookeeper's version (I believe they both release together) | ||
"image": "confluentinc/cp-kafka:6.2.0" | ||
if APPLE_ARM64 | ||
else "confluentinc/cp-kafka:5.1.2", | ||
"ports": {"9092/tcp": 9092}, | ||
"environment": { | ||
"KAFKA_ZOOKEEPER_CONNECT": "{containers[zookeeper][name]}:2181", | ||
|
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 keeping track of the discrepancy. Production uses
3.4.11
.