Implement label-based container lookup for Redis Dev Service

[patrox]

Yet another shameless copy of already existing label-based container discovery logic for Redis to implement: #18029.

Based on these two PRs:

• Kafka: Implement a label based discovery for Kafka Dev Service #17837
• AMQP: Implement shared broker for the AMQP Dev Service #17980

As many of you have noticed this should be refactored - and since I completely agree, I created an attempt to refactor the common code related to label-based container discovery to a separate PR - which at the moment is on my private fork:

https://github.com/patrox/quarkus/pull/3/files

Comments for both changes are very much welcome.

[machi1990]

The code lgtm thanks @patrox for the great work. I'd like @cescoffier and @stuartwdouglas to have a look at this too.

[machi1990]

@patrox do you mind squashing the two commits?

[patrox]

@machi1990 I don't mind it at all, I'll squash them in a sec.

[patrox]

@machi1990 @stuartwdouglas @cescoffier @Ladicek If you review this PR please keep in mind I started to work on a potential improvement in context of label-based container discovery - basically to refactor the code from three different extensions.

The WIP PR is here: patrox#3

I would really appreciate your feedback :)

[machi1990]

The WIP PR is here: patrox#3

I would really appreciate your feedback :)

The WIP PR is great. It goes in direction of what @stuartwdouglas initially suggested as a starting point.
Thanks for also integrating it in other extensions like Kafka and AMQP.

If you review this PR please keep in mind I started to work on a potential improvement in context of label-based container discovery - basically to refactor the code from three different extensions.

Maybe we can open a PR with those improvement in place of this PR. That way, we can review it on its entirely and have it merged, it looked to be in the right direction from my side. What do you think?

[patrox]

@machi1990 That's fine - although initially I wanted to separate them, as the first part (related to implementing a shared container for Redis dev service) is completed, while the second part (related to refactoring) is not completed yet, as AMQP extension requires a bit different result than Kafka and Redis extensions.

On the other hand I think if it will be in one PR, then it will be easier to review, especially since it's based on the first change anyway.

I'll close this PR and open a combined one, if no one objects.

I should be able to handle the remaining changes for AMQP this evening.

[machi1990]

@machi1990 That's fine - although initially I wanted to separate them, as the first part (related to implementing a shared container for Redis dev service) is completed, while the second part (related to refactoring) is not completed yet, as AMQP extension requires a bit different result than Kafka and Redis extensions.

I agree but since you have the changes there anyway, we can combine the two changes and review them together.

On the other hand I think if it will be in one PR, then it will be easier to review, especially since it's based on the first change anyway.

I'll close this PR and open a combined one, if no one objects.

Fully agree on this, it will be easier to review in one go.

I should be able to handle the remaining changes for AMQP this evening.

Awesome, thanks so much for the effort. Let's go this way.

[patrox]

Closed in favor of: #18181