[COOK-1863] Change default service style on Windows to windows_service if Chef >= 11.6 #110
Conversation
| only_if { WMI::Win32_Service.find(:first, :conditions => {:name => 'chef-client'}).nil? } | ||
| # Previous versions of Chef (< 11.6.x) had a windows_service library but no manager. | ||
| # However, they were also broken, so only try this on versions that have the manager | ||
| only_if "chef-service-manager -v" |
There was a problem hiding this comment.
Instead of checking the version I would execute "chef-service-manager" and assume that we're on a good version if it executes. If it's not there exception will be thrown and we can fall back to using winsw.
If I was doing this based on version I would check the version as such:
only_if do
::Chef::VERSION <= "11.6."
More logic to make the version check.
end
There was a problem hiding this comment.
Also I couldn't see the fallback logic to winsw style service installation.
|
Also deprecation or warning message while using winsw with chef version that supports chef-service-manager ? |
| # Will also avoid touching any winsw service if it exists | ||
| execute "register-chef-service" do | ||
| command "chef-service-manager -a install" | ||
| only_if { WMI::Win32_Service.find(:first, :conditions => {:name => 'chef-client'}).nil? } |
There was a problem hiding this comment.
Is there an option where users can specify a different name for the chef-service other than chef-client ?
We should make sure this condition works for winsw + chef-service-manager style installed services.
There was a problem hiding this comment.
Is that a requirement? Given that we've assumed this naming up to now, in both the chef-client cookbook and what the MSI calls it on Windows if you install the service component, I'm a bit loath to change it, or to add that functionality. The code becomes very complicated if we have to account for this?
…o handle release candidates.
…ng on winsw_service, same as in 179e2b2
| end | ||
|
|
||
| # Fall back to winsw on older Chef Clients without the service manager | ||
| if ::Chef::VersionConstraint.new("< 11.6").include?(::Chef::VERSION.split(/\./)[0..1].join('.')) |
There was a problem hiding this comment.
Is the .split necessary here?
There was a problem hiding this comment.
Is this magic due to this only working with x.y or x.y.z versions?
Seth recommends this:
Gem::Requirement.new('< 11.6').satisfied_by?(Gem::Version.new(Chef::VERSION))
That's complicated too. What's the path forward for the best practice for comparing cookbook versions?
…o handle release candidates.
…ng on winsw_service, same as in 179e2b2
@btmspox et. al pointed out that the latter is for cookbook version testing. Note: The use of "11.5" is not a mistake. Using "11.6" will cause RCs to give the wrong behavior; same as hotfixes of the format "11.6.0.hotfix.1". More detail is available in the ticket.
…OK-1863 Conflicts: recipes/windows_service.rb recipes/winsw_service.rb
|
Merged in 0a516b6 |
No description provided.