Systemd support (working) #59
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oh this is just the beginning. FULL SPEED AHEAD!
Next stop, dynamically generate configuration properties.
Instead of defining _ALL_ possible configuration attributes in `attributes/default.rb` we’ll generate key-value pair from whatever attributes that are found under the `broker` namespace. The diffs in this commit are so _HORRIBLE_, but I couldn’t think of any other way to get this done. There are still some tests that needs to be rewritten / added, and I do believe that the configuration generation creates some unnecessary whitespace / newlines that needs to be dealt with, but everything has it’s time, mkay? This is going to be so much easier to maintain.
This commit makes it possible to use either underscored symbol keys for
configuration attributes (i.e. attributes defined under `broker`), or to
use a `dotted` String notation, i.e.
```
node.set.broker = {‘log.dirs’ => %w[/some/dir-1 /some/dir-2]}
```
Decided to get rid of support for using nested Hashes as it’ll become
difficult to render Hash-based options (CSV strings) correctly, and due
to the naming of some of the configuration parameters it’s not possible
to use `node.default.this.attribute = ‘something’` as the parameters
include `default`, `fetch` and whatever else.
Removed a lot of unit tests that are unnecessary now that the broker
configuration file is dynamically rendered.
If there’s a configuration parameter that contains both underscore and dots we don’t want to convert underscores to dots (for obvious reasons).
So, decided to re-introduce the nested Hash notation for accessing configuration attributes, as such: `node.default.broker[:hash][:option] = ‘value’` It does however mean that the “per topic” attributes cannot use Hashes anymore, but either Arrays or just plain Strings.
Just a heads up.
Not used since the configuration rendering is done using helper methods rather than partials.
Because I like it better, mkay?
Was only needed for setting up ZooKeeper with a different configuration file from what Kafka was using.
Was only needed for setting up ZooKeeper with a different configuration file from what Kafka was using.
Previously ZooKeeper and Kafka had different JMX ports, but used the same template for rendering the configuration file. Now that setting up ZooKeeper is out of the picture we don’t need this anymore.
Rather define levels individually for loggers. Don’t forget those trailing commas, yo!
Just a heads up for the peoples. Could be expanded to explain some corner cases and whatnot. [ci skip]
The modules are already namespaced, no need to namespace the individual files as well.
Chef's file cache path is apparently only readable by `root` on OpsWorks, so we have to extract the release archive as root and then manually set ownership.
The (obvious) downside of having too many ways of defining configuration attributes. Perhaps I’ll revise that decision in the future, but for now it’s not too much of a hassle.
Quite a “large” release with a ton of changes, ranging from a rewrite of how “configuration attributes” are managed to minor fixups: * Removal of `zookeeper` recipe. Use a dedicated cookbook for that. * Host all “configuration attributes” under a `broker` namespace. * Configurable and sane log4j properties (see `attributes/default.rb` for examples). * Configurable KAFKA_GC_LOG_OPTS environment variable through the `gc_log_opts` attribute. * Bugfix for using this cookbook with AWS OpsWorks (thanks to @pfleidi).
[ci skip]
Since one can define broker configuration options in a number of different ways we cannot really set broker defaults in an attributes file. Instead we set them in a `_defaults` recipe, but only if they aren’t already set to something, as we don’t want to override what a user has already set. Closes sous-chefs#55.
Bug fix release with a fix for sous-chefs#55.
[ci skip]
Wow, that sucked pretty bad, sorry about that.
Bug fix release for weird default `date_pattern` for log4j appenders.
It’s essentially CentOS, but since I’ve been testing this cookbook on EC2 with instances running Amazon Linux I might as well add it, as it seems like a thing people do.
Basically don’t force-kill the Kafka process if `controlled.shutdown. enable` is set to true. While it’s not necessarily a problem to force- kill the process, it does take quite some time to verify all the logs when it starts up again, and sort of defeats the purpose of even setting the option in the first place. From what I can tell from the upstart documentation it doesn’t try to kill an application if it doesn’t shutdown from the first signal, though I haven’t verified that this is the case.
Thought it would be a lot nicer to actually wait for Kafka to shutdown when `controlled.shutdown.enable` is set to `true`, so that one can use the `restart` action. Unfortunately I had to write a simplified version of `killproc` to actually get this to work. I hate init scripts.
If I understand the documentation for `start-stop-daemon`, giving a `—signal` parameter it won’t force-kill a process.
It'll only apply if you set it. For sous-chefs#56.
Not really necessary. If one set’s it to `false`, it’s their problem.
Definitely not the prettiest tests I’ve written, but they do the job.
First stab at getting `systemd` to work. Still not sure about a few things such as using an EnvironmentFile and the code for setting up everything related to init styles is even messier, so it could probably use some cleanup and rethinking. But hey, it works.
|
Thanks a lot for this, and especially for testing it out, as I've only done some smoke testing with a CentOS 7 virtual box. I cleaned up some things and merged it to master a few minutes ago. |
This was referenced Oct 19, 2014
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was only a very small number of changes that needed to be made to the
templates/default/systemd/kafka.env.erbin order to get this working properly. Additionally the merge conflict from re-basing this branch frommasterwas fixed up. This should be good to merge with no ill effects. The systemd support was tested and validated on CentOS7.