Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

mirrormaker recipe #35

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

jhohertz
Copy link

@jhohertz jhohertz commented Jan 4, 2016

For your consideration, this pull request has one main feature, which is the addition of a recipe to allow an instance of mirror maker to be configured and launched.

In addition, to reduce duplicity in code, the existing recipes were refactored to put the common elements into a common recipe, and the install has been isolated into it's own recipe.

Because we need a second suite to effectively test the setup, this has been added to the test kitchen, which provisions a second kafka instance as well as a mirror. A test has been added that will check for message propagation through the mirror to the second instance. This requires using kitchen verify (which leaves prior suites running) vs. kitchen test (which does not). The alternative is to remove the tests, or allow multiple instances of kafka on a single node, neither of which was deemed appropriate.

Very open to any feedback/changes to get this fork into a condition to merge to the main project. Just let me know what you'd like to see different.

Thanks!

@bbaugher
Copy link
Member

bbaugher commented Jan 6, 2016

Could we point the source and target to the same kafka cluster for testing?

owner node["kafka"]["user"]
group node["kafka"]["group"]
mode 00755
#notifies :restart, "service[kafka]"
Copy link
Member

Choose a reason for hiding this comment

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

We would like to keep this functionality. Heres an example of how to do this with it broken out, https://github.com/bbaugher/apache_zookeeper/blob/master/recipes/configure.rb#L60-L73

Copy link
Author

Choose a reason for hiding this comment

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

this is done

@jhohertz
Copy link
Author

jhohertz commented Jan 6, 2016

Thanks for the comments! I will review and update or prep a new cleaner pull as appropriate, I think some of it is regression as I had split things out well before current... sorry about that.

Re: one cluster for source+target, the complication there I think we'd need some function in mirror maker to prefix or otherwise translate the topic name sourced to a different topic name, which I haven't yet seen. (though it may be there somewhere). It would also need to filter on the same prefix lest a wildcard whitelist create endless loops of mirroring into recursively prefixed topics.

@bbaugher
Copy link
Member

bbaugher commented Jan 6, 2016

I wouldn't worry about an endless loop and point it at some non existing topic. We just want to verify we can configure and run it in our tests

@mkwhitacre
Copy link
Member

@jhohertz can you include adding yourself to the contributors files as well with your updated pull?

[1] - https://github.com/cerner/cerner_kafka#contributing

…un through the list of kafka, kafka-mirror-maker, and kafka-offset-monitor as any could be affected by the change if present.
… that did not make sense removed. suggested in pull cerner#35
…ust JMX_PORT into the init script, letting them be overridden from the ones in the bash profile as used by kafka brokers (used prior this commit for mirror too)
@jhohertz
Copy link
Author

jhohertz commented Jan 9, 2016

All the minor stuff is in-branch now. What I have left to address:

  • tests / kitchen simplification (one suite)
  • pull the config assertions out?
  • attribute namespace changes (this will be a little while yet as I'll need to be updating our own wrappers/configs in tandem)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants