-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
provisioner/chef: Support named run-lists for Policyfiles #11215
provisioner/chef: Support named run-lists for Policyfiles #11215
Conversation
Add an optional argument for overriding the Chef Client's initial run with a named run-list specified by the Policyfile. This is useful for bootstrapping a node with a one-time setup recipe that deviates from a policy's normal run-list.
I'm brand new to Go so I haven't the slightest clue why 1.8beta2 is failing and not 1.7, but I'll try to sort out the Travis failure. Any suggestions much appreciated! |
Hi @kpersohn Sorry for the alarm here - the build issue was independent of this PR Paul |
cmd = fmt.Sprintf("%s -j %q", chefCmd, fb) | ||
} else { | ||
cmd = fmt.Sprintf("%s -j %q -n %q", chefCmd, fb, p.NamedRunList) | ||
} | ||
} else { | ||
cmd = fmt.Sprintf("%s -j %q -E %q", chefCmd, fb, p.Environment) | ||
} |
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.
I think it would be more idiomatic Go (and a little cleaner to read) to write this as follows:
// Policyfiles do not support chef environments, so don't pass the `-E` flag.
switch {
case p.UsePolicyfile && p.NamedRunList == "":
cmd = fmt.Sprintf("%s -j %q", chefCmd, fb)
case p.UsePolicyfile && p.NamedRunList != "":
cmd = fmt.Sprintf("%s -j %q -n %q", chefCmd, fb, p.NamedRunList)
default: }
cmd = fmt.Sprintf("%s -j %q -E %q", chefCmd, fb, p.Environment)
}
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.
I agree, that's cleaner. Thanks for the suggestion!
@kpersohn thanks for the PR! I have one suggestion to make the code a little more idiomatic, but other then that this looks good 👍 |
LGTM! Thanks again @kpersohn |
* master: (232 commits) Update CHANGELOG.md command/plugin_list: Adding Alicloud to the plugin list file (#11292) Additionnal information for machine type (#11288) provider/alicloud: Creating the necessary structure for the alicloud documentation (#11289) Update CHANGELOG.md update alicloud provider (#11235) Update CHANGELOG.md provisioner/chef: Support named run-lists for Policyfiles (#11215) Correct data.aws_route_table filter AWS docs link Update CHANGELOG.md Update CHANGELOG.md removes region param from google_compute_backend_service (#10903) core: Add pathexpand interpolation function Update CHANGELOG.md Add instance_tags as an additional filter provider/aws: Add aws_instance data source Update `parameter_group_name` (#11269) provider/profitbricks: Rename the profitbricks bin so that the plugin (#11267) Update CHANGELOG.md Terraform ProfitBricks Builder (#7943) ...
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Add an optional argument for overriding the Chef Client's initial
run with a named run-list specified by the Policyfile. This is useful
for bootstrapping a node with a one-time setup recipe that deviates
from a policy's normal run-list.