Skip to content
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

Adds formatter option to Chef Provisioner #1250

Closed

Conversation

miketheman
Copy link
Contributor

Adds the ability to specify in the Vagrantfile a formatter directive to control the verbosity/format of the Chef log.

Formats such as summary, minimal and doc are valid, and the directive is not required.

I tried taking a stab at the tests, but the test suite didn't seem very friendly to me today.

@StephenKing
Copy link
Contributor

+1, this would be great!

@miketheman
Copy link
Contributor Author

@mitchellh any feedback on this? Came up again as something I was curious about using today.

@mitchellh
Copy link
Contributor

@miketheman It looks good but I'm not accepting patches back against 1.0 anymore. If you get this to work again 1.x I'll merge right away. Thanks!

@mitchellh mitchellh closed this Jul 18, 2013
@miketheman
Copy link
Contributor Author

Sure thing. This was submitted while 1.1/2 were still unreleased.
I'll port to the new master branch, and test.

miketheman added a commit to miketheman/vagrant that referenced this pull request Jul 18, 2013
mitchellh added a commit that referenced this pull request Jul 18, 2013
Adds formatter option to Chef Provisioner [GH-1250]
@@ -28,5 +28,7 @@ https_proxy_pass <%= https_proxy_pass.inspect %>
no_proxy <%= no_proxy.inspect %>

pid_file "/var/run/chef/chef-client.pid"

<% if formatter %>
formatter "<%= formatter %>"
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be add_formatter because formatter does not work :-(

see http://lists.opscode.com/sympa/arc/chef/2013-08/msg00071.html

@StephenKing
Copy link
Contributor

Ah, that's why it didn't work when I have it a try couple of days ago..

@miketheman
Copy link
Contributor Author

I'm confused - formatter works for chef-client, but not chef-solo?

@tknerr
Copy link
Contributor

tknerr commented Aug 6, 2013

That's really strange if it works with chef-client. I have tested with chef-solo only...

@tknerr
Copy link
Contributor

tknerr commented Aug 7, 2013

@miketheman is this chef-client 10.x or 11.x where formatter works for you?

@danielsdeleo mentioned on the chef list that this could be the issue.

@miketheman
Copy link
Contributor Author

@tknerr Just spent some time verifying this.
Sure enough, the behavior of formatter was the one added in 10.x, and is broken in 11.x, needing add_formatter.
However, add_formatter is incompatible with 10.x, and any formatter output from Chef is lost.

A total kludge would be to add both statements into the config file, and I'm not a fan of that - it's grosstastic.

I think The Right Thing would be to use the add_formatter syntax to support Chef 11, and the Chef 10.x users will simply not have this feature.

Thoughts?
/cc: @danielsdeleo

miketheman added a commit to miketheman/vagrant that referenced this pull request Aug 16, 2013
Refs hashicorp#1250.

Will correctly produce the desired result on Chef 11.x and above, as the
original directive was written against Chef 10.x, and released in Vagrant 1.2.7.

While this will continue to work for Chef 10.x, since this is not the 'mainline'
release track, and the alternative would be to write more conditional code in
the configuration file to detect the version and place the correct directive, this
changes the directive to support the current releases of Chef 11 and above.
@tknerr
Copy link
Contributor

tknerr commented Aug 16, 2013

@miketheman can we conditionally include either the one or the other? Do we know the Chef version here?

@danielsdeleo is 'add_formatter' likely to stick past Chef 11

@tknerr
Copy link
Contributor

tknerr commented Aug 16, 2013

I'm 👍 for Chef 11 otherwise :-)

@miketheman
Copy link
Contributor Author

@tknerr I don't think the version comes through to the provisioner, and adding that seems like the kludge I mentioned earlier.

@tknerr
Copy link
Contributor

tknerr commented Aug 16, 2013

@miketheman yeah, having both statements in the rendered template would be kludgy. The only way I could think of would be to shell out to 'knife -v' while the template is rendered (assuming it's rendered on the guest vm), but that's probably not much better...

Chef 11 would be fine for me

@miketheman
Copy link
Contributor Author

Great. @mitchellh @fgrehm this is ready to whenever you want to press the big green button.

@fgrehm
Copy link
Contributor

fgrehm commented Aug 20, 2013

@miketheman Cool, I'm heading on vacation today for a week, if Mitchell is not able to handle it feel free to ping me by then ;)

@ghost ghost locked and limited conversation to collaborators Apr 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants