-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Node JSON support #101
Node JSON support #101
Conversation
In chef nodes can be passed individual json snippets, called node json, to customize certain things per node. For example host names, node ids and similar. This commit adds node json support to stemcell. jq is like sed for json, is written in c and has no runtime dependencies. Packages exist in the official repositories for Debian, Ubuntu, Fedora, openSUSE, Arch and Solaris (https://stedolan.github.io/jq/).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
besides the nits here, is further parametrizing chef a desirable property? currently, you can say that what chef does on an instance is a function of it's role, branch, and environment, and these are simple strings.
of the reasons mentioned for adding node_json, the only one that really makes sense to me is software license keys. is that an actual use case you'd like? what is the actual reason that this is needed?
@@ -48,6 +49,7 @@ domain_name='<%= opts['instance_domain_name'] %>' | |||
chef_version='<%= opts['chef_version'] %>' | |||
username='<%= opts.fetch('user', ENV['USER']) %>' | |||
encrypted_data_bag_secret_path='<%= opts['chef_data_bag_secret_path'].to_s %>' | |||
node_json='<%= opts['node_json'] %>' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the quoting would break if the node json contains '
characters. this requires a more serious escaping strategy -- a HEREDOC might to the trick.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I've added a commit that solves this.
lib/stemcell/launcher.rb
Outdated
@@ -181,6 +182,14 @@ def launch(opts={}) | |||
end | |||
end | |||
|
|||
if opts['node_json'].is_a?(Hash) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you expect this to be a hash, or a string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both, but I agree that it wasn't very clear!
The old code would (1) turn a hash into JSON string, (2) allow a string (any string) and (3) if no string was set in step (1) or passed in, it would default to an empty JSON object.
I've added an amend commit which improves this: 8c06c9b
@@ -226,7 +232,9 @@ GIT_SSH=${git_wrapper} git fetch | |||
git reset --hard origin/\${branch} | |||
git clean -fdx | |||
|
|||
json="{\"role\": \"\${role}\", \"env\": \"\${env}\", \"branch\": \"\${branch}\"}" | |||
base_json="{\"role\": \"\${role}\", \"env\": \"\${env}\", \"branch\": \"\${branch}\"}" | |||
json=\`echo \$node_json \$base_json | jq -s '.[0] + .[1]'\` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's no reason to involve an external binary; you can just construct the variable you want in ruby and pass it, fully constructed, into the template.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, I've added a commit which moves this over to 'ruby-land'.
@igor47 First of all, thanks for reviewing! 😄 Much appreciated! I've addressed your points in new commits. Let me know if you have more comments or suggestions. Regarding the use-case, I'd say there are a few:
I realize that everyone may not have these uses, but node-json is part of Chef itself and commonly used. This would also lessen the need for wider cloud-init support (since many of the things that cloud-init solves can also be solved via Chef node-json). Also, great if you'd like to take a look at #100 too. |
@igor47 I've run across an issue related to node-data, but for environment. On Is this an old way of setting the env in chef, or something custom? Afaik it should be set via the Also, the runlist/role/env dance, is that something custom?
I've added a commit here that should fix this issue 4a08273, let me know what you think! I can also open a separate PR for that, but it's related to custom json so easier to track here if possible. |
@igor47 friendly ping 😄 |
Hey @igor47, do you have a few minutes to look at this? |
In chef nodes can be passed individual JSON snippets, called node JSON. Node JSON is used to customize certain things per node.
Some examples are host names, unique node ids, unique tokens for nodes to 'phone home' and report that they are setup successfully (for load-balancers registration, etc), adding per-node unique software license keys, data to seed the random generator and much more.
This commit adds node-JSON support to stemcell.
jq
is likesed
for JSON, is written in c and has no runtime dependencies. Packages exist in the official repositories for Debian, Ubuntu, Fedora, openSUSE, Arch and Solaris (https://stedolan.github.io/jq/). The footprint is small, it clocks in at ~50 kb.@darnaut Hi Davi, I'm eager to hear your thoughts on this!
Closes #93