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

Use docker exec instead of docker cp to create run script for Kafka native DevService #38790

Merged
merged 1 commit into from
Feb 16, 2024

Conversation

xstefank
Copy link
Member

This also requires ozangunalp/kafka-native#144 to be pushed to quay.io. cc/ @ozangunalp

With this change I was able to successfully run Kafka native DevService in DevSpaces/Che!

config

quarkus.kafka.devservices.provider=kafka-native

This would really help us to get the Quarkus super-heroes workshop to be doable in DevSpaces (fully online).

cc/ also @danieloh30 since I heard that this was a blocker for you.

@ozangunalp
Copy link
Contributor

@xstefank that string format is hard to follow, but it looks good!

Can we do the same for the redpanda container?

@ozangunalp
Copy link
Contributor

@xstefank also, the change itself doesn't require the kafka-native release right? We can cut it after?

Also you can test with the kafka-native latest-snapshot tag that is built & pushed from the main.

This comment has been minimized.

@xstefank
Copy link
Member Author

@ozangunalp redpanda has itself similar issue as we fixed in your kafka-native container (permissions for random UID) so until they fix it in their container, we can't do anything on our side.

Add release, yes, this is just a different way to create run.sh script in container so it shouldn't matter. But I see test failures so probably it does?

@ozangunalp
Copy link
Contributor

KafkaDevServicesDevModeTestCase.sseStream is a failing test, not even flaky apparently. I am looking at it. So it is irrelevant for this change.

// docker exec since docker cp doesn't work with kubedock yet
try {
execInContainer("sh", "-c",
String.format("echo -e \"%s\" >> %s && chmod 777 %s", cmd, STARTER_SCRIPT, STARTER_SCRIPT));
Copy link
Member

Choose a reason for hiding this comment

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

The following might be a little less confusing (untested):

Suggested change
String.format("echo -e \"%s\" >> %s && chmod 777 %s", cmd, STARTER_SCRIPT, STARTER_SCRIPT));
String.format("echo -e \"%1$s\" >> %2$s && chmod 777 %2$s", cmd, STARTER_SCRIPT));

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks

@xstefank xstefank force-pushed the kafka-native-kubedock branch from 51162c6 to 079b2b7 Compare February 15, 2024 12:30
Copy link

quarkus-bot bot commented Feb 15, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 079b2b7.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@geoand geoand merged commit 1092bbc into quarkusio:main Feb 16, 2024
23 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.9 - main milestone Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants