Paramaterize script list

[justizin]

I'd like to install more scripts than this, but I'm happy to set that in a wrapper cookbook or via node / override attributes.

This change results in the same output as the previous code, but allows for a list of scripts to copy to be configured by others with no code change.

[justizin]

Ah, this doesn't work for adding, I'll need to amend it.

Edit: looked at first as if files from the distribution were simply packaged in the cookbook without being templates, but I see that kafka-server-start.sh.erb and kafka-topics.sh.erb use the bin_dir variable.

I'll put some thought into this and update the PR. It would be nice to absorb changes to the scripts from upstream, but it also feels sinful to use sed in a chef run. :)

[mathyourlife]

Thanks Justin.

Moving the list of scripts to a node attribute is a good idea.

Unfortunately, the scripts that come packaged with the distribution both
set environment variables and launch the application. It seemed safer/more
stable to separate out the variables allowing for overrides and create a
simplified script. You're correct though that it would not benefit from
upstream changes. I'd be happy to consider alternatives. I'm trying to
keep the cookbook as straight forward as possible.

Dan
On Jun 10, 2015 3:52 PM, "Justin Alan Ryan" [email protected]
wrote:

Ah, this doesn't work for adding, I'll need to amend it.

It seems at a glance like the cookbook packages scripts that are part of
the kafka distribution, and can simply be copied from the extracted
distribution - unless I'm not catching the actual use of template variables.

—
Reply to this email directly or view it on GitHub
#12 (comment)
.

[justizin]

Thanks for the feedback, Dan!

It really seems the only use of these variables is to locate kafka-run-class.sh. In the current source distribution I'm getting, it sets $base_dir based on the location of the script, so this may now be addressed upstream.

In the current change, I have two node attrs: install_scripts and template_scripts.

In the 'install' recipe, by default currently only kafka-run-class.sh is installed this way, though in my wrapper cookbook I have listed all of the scripts. I'm not actually sure there's any good reason not to just copy all of the bin/*.sh from the dist into /usr/local/kafka/bin, but I wanted this to be a small change.

You can see my usage at https://github.com/bitmonk/bitmonk_kafka (vagrant up will get you a 3-node zk/kafka cluster, but without all the shell scripts, it's harder for me to test, since we don't have an app using kafka yet).

Happy to continue adapting it for the sake of an upstream merge, for now my branch is usable for me, but I hate running forked code, not the least because if it's not good enough for the community, it's not really good enough for me.

[mathyourlife]

I like it.

If that /etc/default/kafka is still sourced prior to the bin scripts most of the variable setting will be skipped and still allow users to override with node attributes. I'm in the middle of something right now, but I can probably take a closer look at this tonight.

Thanks again Justin

[mathyourlife]

Ran into a few issues

• config directory doesn't exist yet
• log directory reverted back to the $base_dir/logs since current env doesn't have LOG_DIR

Think adding

    action :nothing
    subscribes :create, "execute[unzip kafka source]", :immediately

can prevent rewriting files on each chef run and only execute the copy on extracting a new version.

[justizin]

I was running into problems with the scripts I installed, e.g. kafka-console-producer.sh, and doing a 'source /etc/default/kafka' in my shell fixes it for testing purposes. Not sure what the best solution to this is, going to chew on it this morning, but thanks for mentioning that.

Not sure what you mean by, 'config directory doesn't exist yet', maybe we're using the cookbook slightly differently? Nope, I think this is fixed in the latest change.

Agree on the proposed change to avoid rewriting files, etc..

[justizin]

I had to fix a bug related to named resources declared inside a loop, but it looks like it's avoiding repeat copies now!