Suggestion: Add image name to kafka preset

[johanhugg]

Since this image for kafka doesn't really work with M1 (yet) there has been a need to override the default image name

Good idea or not to have this in the main repo?

[orlangure]

Hi @johanhugg, I like this idea very much 😼 Do you have a particular image that works on arm64?

If yes, and if that image is compatible with the existing preset behavior (healthchecks, ports, all that), I'd like to use it on arm64 architecture even without a custom option, and the regular image on any other architecture.

If the image exists but is not compatible with whatever we have in this preset, we can have 2 separate preset implementations, one for arm64 and one for everything else (again, only in case that the entire preset needs to be reimplemented for that other image).

What do you think?

[codecov-commenter]

Codecov Report

Base: 85.97% // Head: 85.78% // Decreases project coverage by -0.18% :warning:

Coverage data is based on head (4e14509) compared to base (d8fc13b). Patch coverage: 50.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #656      +/-   ##
==========================================
- Coverage   85.97%   85.78%   -0.19%     
==========================================
  Files          49       49              
  Lines        2267     2272       +5     
==========================================
  Hits         1949     1949              
- Misses        165      169       +4     
- Partials      153      154       +1     

Impacted Files	Coverage Δ
preset/kafka/options.go	80.00% <0.00%> (-20.00%)	:arrow_down:
preset/kafka/preset.go	79.20% <100.00%> (-1.29%)	:arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[johanhugg]

Hi,

The image I used to test is this one linked in the PR: https://github.com/lensesio/fast-data-dev/pull/185 It works fine as far as I can tell with gnomock, I didn't have to change anything else. However to me it doesn't seem like a good idea to use that image he published to test this since it will become deprecated later whenever that PR is merged.

[ahjashish]

I basically got it working with creating a wrapper around the default Preset interface used by kafka preset and then implementing custom image function that returns the Apple M1 compatible image until this PR is merged.

type CustomKafkaPreset struct {
	k gnomock.Preset
}

func NewCustomKafkaPreset(p gnomock.Preset) *CustomKafkaPreset {
	return &CustomKafkaPreset{k: p}
}

func (c *CustomKafkaPreset) Image() string {
	return "docker.io/dougdonohoe/fast-data-dev:latest"
}

func (c *CustomKafkaPreset) Ports() gnomock.NamedPorts {
	return c.k.Ports()
}

func (c *CustomKafkaPreset) Options() []gnomock.Option {
	return c.k.Options()
}

func TestConfluentPlatformStart(t *testing.T) {
	const kafkaContainerName = "kafka"
	const kafkaTopicName = "kafka-topic"

	kc, err := gnomock.Start(
		NewCustomKafkaPreset(
			kafka.Preset(kafka.WithTopics(kafkaTopicName))),
		gnomock.WithDebugMode(), gnomock.WithLogWriter(os.Stdout),
		gnomock.WithContainerName(kafkaContainerName),
	)
	require.NoError(t, err)

	defer func() {
		require.NoError(t, gnomock.Stop(kc))
	}()
}

[orlangure]

@johanhugg, I agree that there is no point in using that image by default, and in this case it also makes little sense to make the image configurable externally. I like the way @ahjashish suggests to solve the issue by wrapping the preset and only changing the returned image. Do you think this solution can work for you, or do you still think that making the image easily configurable would be much better?

Regardless of what we choose to do, I'm glad you brought it up, because now people that struggle with Kafka preset on arm64 instances/macbooks will have a solution😼

[johanhugg]

@johanhugg, I agree that there is no point in using that image by default, and in this case it also makes little sense to make the image configurable externally. I like the way @ahjashish suggests to solve the issue by wrapping the preset and only changing the returned image. Do you think this solution can work for you, or do you still think that making the image easily configurable would be much better?

Regardless of what we choose to do, I'm glad you brought it up, because now people that struggle with Kafka preset on arm64 instances/macbooks will have a solution😼

Yeah the solution @ahjashish did will work for us just fine, nice way of solving it! I think it should be fine to just close this, I made a fork that we use at our company for now, but will probably use the wrapper solution as mentioned until the PR in the fast-data-dev repo is merged.