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

feat(risedev): use docker for kafka service #16536

Merged
merged 12 commits into from
May 24, 2024
Merged

Conversation

BugenZhao
Copy link
Member

@BugenZhao BugenZhao commented Apr 29, 2024

Signed-off-by: Bugen Zhao i@bugenzhao.comI hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

See #16903 for the background.

Use docker image confluentinc/cp-kafka to start Kafka in RiseDev.

Checklist

  • I have written necessary rustdoc comments
  • I have added necessary unit tests and integration tests
  • All checks passed in ./risedev check (or alias, ./risedev c)

Documentation

  • My PR needs documentation updates. (Please use the Release note section below to summarize the impact on users)

Release note

If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.

Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
@BugenZhao BugenZhao changed the title feat(risedev): deprecate zookeeper & use docker w/ kraft for kafka service feat(risedev): use docker for kafka service May 21, 2024
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
@BugenZhao BugenZhao marked this pull request as ready for review May 24, 2024 08:06
@kwannoel kwannoel added the ci/run-backwards-compat-tests Run backwards compatibility tests in your PR. label May 24, 2024
Copy link
Member

@xxchan xxchan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

e2e_test/source_inline/commands.toml Outdated Show resolved Hide resolved
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Copy link
Contributor

@stdrc stdrc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks much cleaner! Since kafka can not be gracefully killed with ease, seems that we can bring double risedev d back again?

fn kafka(&self) -> Result<Command> {
Ok(Command::new(self.kafka_path()?))
fn image(&self) -> String {
"confluentinc/cp-kafka:7.6.1".to_owned()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to have a place define all the versions of such docker images? Also is it possible to clean old versions of these images if we later upgrade the version?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to have a place define all the versions of such docker images?

You reminded me that the image name should be customizable through risedev.yml.

Also is it possible to clean old versions of these images if we later upgrade the version?

We cannot tell if the image is pulled by RiseDev or the user, so let's just leave it to the user.

Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
@BugenZhao BugenZhao added this pull request to the merge queue May 24, 2024
Merged via the queue into main with commit e5ad5d9 May 24, 2024
28 of 29 checks passed
@BugenZhao BugenZhao deleted the bz/risedev-docker-kafka branch May 24, 2024 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/run-backwards-compat-tests Run backwards compatibility tests in your PR. type/feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants