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

Improve ftr license agreement #199

Merged

Conversation

Joshua-Snapp
Copy link
Contributor

ftr stands for "first time run"

Current

Triggering the exec resource based on the presence of the etc/auth/server.pem prevents the exec resource from being called after splunk has been upgraded, unless we're willing to delete the server.pem file.

Suggested

The ftr file is an improved trigger for determining when to run the license_splunkforwarder exec resource. The ftr file always exists immediately following an install of splunk or splunkforwarder. The install could be a first time install or an upgrade. Once the license agreement has been accepted the ftr file is removed.

This Pull Request (PR) fixes the following issues

Upgrades don't trigger license_splunkforwarder exec resource #198

@igalic
Copy link
Contributor

igalic commented Sep 28, 2018

@Joshua-Snapp you may want to add Joshua Snapp <jksnapp@ncsu.edu> to your github emails, or else change the commits so you're properly credited!

ftr stands for first time run.
Running splunk ftr doesn't start the splunk service. It just allows for
accepting the license agreement. Then we can let the splunk service
resource handle whether or not the state of the service should actually
change.
ftr stands for first time run.

The ftr file is an improved trigger for determining when to run the
license_splunkforwarder exec resource. The ftr file always exists
following an install of splunk. The install could be a first time
install or an upgrade. The ftr file is removed when the license
agreement has been accepted.

Triggering the exec resource based on the presence of the
etc/auth/server.pem prevents the exec resource from being called after
splunk has been upgraded, unless we're willing to delete the server.pem
file.
@Joshua-Snapp Joshua-Snapp force-pushed the improve_ftr_license_agreement branch from 887ff95 to e9cca9c Compare September 29, 2018 04:37
@Joshua-Snapp
Copy link
Contributor Author

@igalic I appreciate you catching my email address. I just corrected it in both commits.

user => $splunk_user,
creates => '/opt/splunkforwarder/etc/auth/server.pem',
onlyif => "/usr/bin/test -f ${splunk::params::forwarder_dir}/ftr",
Copy link
Member

Choose a reason for hiding this comment

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

Can't we use creates => "/usr/bin/test -f ${splunk::params::forwarder_dir}/ftr" here?

@Joshua-Snapp
Copy link
Contributor Author

We can't use creates if we want this to be idempotent. I'm suggesting we pay attention to the presence of the "first time run" ftr file that appears as a result of an install or uprade. That file is deleted as soon as the --accept-license option is used. We want to run the license_splunkforwarder exec resource onlyif the ftr file exists. As soon as we accept the license, the file is removed. Any time the forwarder is upgraded, the ftr file appears again. As a result, the license_splunkforwarder exec resource should run the --accept-license option, which deletes the ftr file.

You can see the behavior of the ftr file by simply installing the forwarder and listing the contents of /opt/splunkforwarder directory before you accept the license. Then take a look at the contents after you accept the license. The way this ftr is only temporarily present based on license acceptance makes it ideal as a trigger in this case.

With this code change, I've been able to install forwarder 6.2.5 and upgrade to 6.6.8 and again to 7.1.2 by simply changing version and build values. Puppet successfully downloads the rpm, upgrades the forwarder and accepts the license allowing the service to start.

Take the following code and install 6.6.8. Then comment out 6.6.8 version and build and uncomment 7.1.2 version and build. Run puppet again and watch forwarder get updated and license accepted.

  $_splunk_options = { 
    version => '6.6.8',
    build => '6c27a8439c1e',
    #version => '7.1.2',
    #build => 'a0c72a66db66',
  } 

  # Tell the module to get packages directly from Splunk.
  class { '::splunk::params':
    version => $_splunk_options['version'],
    build => $_splunk_options['build'],
    src_root => 'https://download.splunk.com',
  } 

  # Specify version and build of package to be installed.
  class { '::splunk::forwarder':
    package_ensure => ($enable_forwarder_param) ? { 
      true  => "$_splunk_options['version']-$_splunk_options['build']", 
      false => absent,
    }
  } 

@bastelfreak bastelfreak added the bug Something isn't working label Sep 30, 2018
@bastelfreak
Copy link
Member

Thanks for the explanation!

@Joshua-Snapp
Copy link
Contributor Author

No problem. I'd actually like to see something like the last sentence and code example in my last comment added to the README. I think it would make the upgrade path a lot more clear.

Where I work, we've struggled to provide a way to easily upgrade the forwarder on hundreds of machines. Having just figured this out, we're going to add an extra exec resource to our Splunk profile to run the --accept-license option to perform the same action that this PR is recommending. This puppet-splunk component module already allows us to specify the version and build. So, whether this pull request gets merged or not, we now understand how to easily handle the forwarder upgrade by simply changing the version and build values.

@bastelfreak
Copy link
Member

ah lol, I wanted to merge it this morning but got distracted. Now that you mentioned it, could you add the example to the README.md?

@Joshua-Snapp
Copy link
Contributor Author

I just added an extra commit. It's not the README yet. I'll do that with whatever content is appropriate based on a conversation about this most recent commit.

The recent commit changes the server_pkg_ensure and forwarder_pkg_ensure values in ::splunk::params from 'installed' to "${version}-${build}". This way, the end user of the module doesn't have to specify version and build values for ::splunk::params and ::splunk::forwarder, like my example in a previous comment in this PR shows. They can just specify it in one place.

    # Tell the module to get packages directly from Splunk.
    class { '::splunk::params':
      version => '7.1.2',
      build => 'a0c72a66db66',
      src_root => 'https://download.splunk.com',
    }   

    include ::splunk::forwarder

Let me know what you think. I understand it might be argued that it's out of scope with the other work. But it also could be considered a good time to get this in since the goal of this PR is to make this module capable of upgrading the Splunk packages.

@Joshua-Snapp Joshua-Snapp force-pushed the improve_ftr_license_agreement branch from 02df5ed to 36233c5 Compare October 1, 2018 17:15
@Joshua-Snapp Joshua-Snapp force-pushed the improve_ftr_license_agreement branch from 36233c5 to 47bc08c Compare October 1, 2018 17:18
@Joshua-Snapp
Copy link
Contributor Author

@bastelfreak Sorry for the confusion and delay in finishing the work here. Please ignore my last comment and the work I described.

The short story is that a system admin only needs to set package_ensure => 'latest' for the ::splunk or ::splunk::forwarder class that they are declaring. Doing that will allow them to upgrade the associated package by simply changing to newer version and build values. I added this explanation and a code example to the README. This has been tested on CentOS 7 and Ubuntu 16.04.

Let me know if this makes sense or if you have any questions.

@Joshua-Snapp
Copy link
Contributor Author

@bastelfreak
Do you have time to look at this PR? I think it has what you requested. If possible, I'd like to see it get merged before your next version release.

@bastelfreak bastelfreak merged commit 6b6e981 into voxpupuli:master Oct 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants