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.

Sign up for GitHub

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

Jump to bottom

Add support for log rotate on Windows #607

Merged
merged 9 commits into from
May 8, 2018
Merged

Add support for log rotate on Windows #607

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c82842b
support for windows
evandervecht Apr 16, 2018
0522692
.
evandervecht Apr 16, 2018
edc8b63
string to int
evandervecht Apr 18, 2018
5edd695
each os version should point to their own repo url
evandervecht May 7, 2018
c480483
support for windows
evandervecht Apr 16, 2018
7a31c2e
string to int
evandervecht Apr 18, 2018
1756c52
each os version should point to their own repo url
evandervecht May 7, 2018
8ee4564
Merge branch 'develop' of github.com:ridiculousness/sensu-chef into d…
evandervecht May 8, 2018
4192ead
default msi url back to attributes files, only distinguish in os version
evandervecht May 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions attributes/default.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@
end

# installation
default["sensu"]["version"] = "0.28.4-1"
default["sensu"]["version"] = "1.2.0-1"
default["sensu"]["version_suffix"] = nil
default["sensu"]["apt_repo_codename"] = nil
default["sensu"]["yum_repo_releasever"] = nil
default["sensu"]["use_unstable_repo"] = false
default["sensu"]["log_level"] = "info"
default["sensu"]["log_rotate_file_size"] = '10240'
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on your screenshot and the documentation example this should be an integer not a string but the puppet module has a string. I am leaning integer because thats what the documentation has but not sure matters 🤷‍♂️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@majormoses Thanks for the review. I changed it to int based on your comment/docs. It should not really matter for the function of it but this way it's consistent.

default["sensu"]["log_rotate_file_keep"] = '10'
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on your screenshot and the documentation example this should be an integer not a string but the puppet module has a string. I am leaning integer because thats what the documentation has but not sure matters 🤷‍♂️

default["sensu"]["use_ssl"] = true
default["sensu"]["use_embedded_ruby"] = true
default["sensu"]["service_max_wait"] = 10
Expand All @@ -34,7 +36,7 @@
default["sensu"]["apt_repo_url"] = "http://repositories.sensuapp.org/apt"
default["sensu"]["yum_repo_url"] = "http://repositories.sensuapp.org"
default['sensu']['yum_flush_cache'] = nil
default["sensu"]["msi_repo_url"] = "https://repositories.sensuapp.org/msi"
default["sensu"]["msi_repo_url"] = "https://repositories.sensuapp.org/msi/2012r2"
Copy link
Contributor

Choose a reason for hiding this comment

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

why does this need to change? I am still unclear with the previous discussion wont this break all other versions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@majormoses When you go to the old url, version go up to 0.26. Logrotate works as of 0.29.x. Puppet version also uses the new url, however still using the 0.29.x version. If uncertain of my comment please check the old url vs new url.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see new versions in here: http://repositories.sensuapp.org/msi/2016/ which was my point the windows version plays into what is installed. I really don't know enough about windows stuff but this feel like this would break every version other than server 2012.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the problem is that we need to include the version not hardcore it to 2012r2

I don't have a windows machine to test with can you please give me the relevant ohai output for this: https://github.com/chef/ohai/blob/v14.1.0/lib/ohai/plugins/windows/platform.rb#L30 as I think we can use this to solve the problem.

Copy link
Contributor Author

@evandervecht evandervecht May 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@majormoses I can use the current Windows cookbook (1.36) using on platform_version.
I'd rather use "case ::Windows::VersionHelper.nt_version node", which is more clean than the current solution, however to use that I need to bump up the Windows coookbook version to > 4.0.0 which might cause some cookbook rewriting. To have the least impact on the dependencies I used the current platform_version.

I also removed the attribute from the attributes file and placed the logic in the recipe. Please let me know what you think of it. As with the logic it already checks which version is needed I do not think it's necessary to have it in the attributes file (it also excludes saving the attribute on Linux hosts).

default["sensu"]["aix_package_root_url"] = "https://sensu.global.ssl.fastly.net/aix"
default["sensu"]["add_repo"] = true
default['sensu']['apt_key_url'] = 'https://sensu.global.ssl.fastly.net/apt/pubkey.gpg'
Expand Down
8 changes: 7 additions & 1 deletion recipes/_windows.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@

windows = node["sensu"]["windows"].dup

if node['kernel']['machine'] =~ /x86_64/
kernel = 'x64'
else
kernel = 'x86'
end

user node["sensu"]["user"] do
password Sensu::Helpers.random_password(20, true, true, true, true)
not_if { Sensu::Helpers.windows_user_exists?(node["sensu"]["user"]) }
Expand All @@ -35,7 +41,7 @@
end

package "Sensu" do
source "#{node['sensu']['msi_repo_url']}/sensu-#{node['sensu']['version']}.msi"
source "#{node['sensu']['msi_repo_url']}/sensu-#{node['sensu']['version']}-#{kernel}.msi"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be "#{node['sensu']['msi_repo_url']}/#{version}/sensu-#{node['sensu']['version']}-#{kernel}.msi"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can do, but it would mean that the OS version is always part of the url.

If we keep it like this, anybody can overwrite the #{node['sensu']['msi_repo_url']} attribute and point it to a different url also without the OS version bound in the url itself.
For example, I cannot overwrite the attribute anymore with www.google.com/download/ if the client is located at www.google.com/download/sensu-client-x64.msi. I then have to save the file in www.google.com/download//sensu-client-x64.msi, for each OS that I have 1 time.

A reason to save/cache the binary somewhere else can be, if the original url changes multiple times or the installed version is removed from the original url.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair point but that's true about the kernel version as well, they could rename the file because they feel they don't need the (x86|x|64) version. I feel like if they want to use a different (internal?) location they should mirror the repo structure and copy several versions over (for rollback) not just copy the one binary they want. By making it single attribute it is harder to override for just a particular instance. By just needing to give it the base path and version it should figure out the platform and pull from the right location. In my experience most shops do not have all servers with the exact same OS version especially in windows land as things tend to be more manual and therefore servers are often left on older versions of windows server while newer systems are put on newer versions.

Copy link
Contributor Author

@evandervecht evandervecht May 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

True that.

We use an internal artifact system but can't remember the exact reason why :-). Currently, it's being used by ~20 customers of us.

I updated the recipe/attributes files. I already had it ready :-)

When I bump up the cookbook I will also need to update the Artifacts repo, but that's not a big issue.

options windows["package_options"]
version node["sensu"]["version"].tr("-", ".")
notifies :create, "ruby_block[sensu_service_trigger]", :immediately
Expand Down
7 changes: 5 additions & 2 deletions templates/default/sensu.xml.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,11 @@
<argument><%= node["sensu"]["directory"] %>\conf.d</argument>
<argument>-e</argument>
<argument><%= node["sensu"]["directory"] %>\extensions</argument>
<argument>-l</argument>
<argument><%= node["sensu"]["log_directory"] %>\<%= @service %>.log</argument>
<argument>-L</argument>
<argument><%= node["sensu"]["log_level"] %></argument>
<logpath><%= node["sensu"]["log_directory"] %>\</logpath>
<log mode="roll-by-size">
<sizeThreshold><%= node["sensu"]["log_rotate_file_size"] %></sizeThreshold>
<keepFiles><%= node["sensu"]["log_rotate_file_keep"] %></keepFiles>
</log>
</service>