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

[WIP] Sensu Enterprise & Enterprise Dashboard support #400

Merged
merged 1 commit into from
Aug 24, 2015

Conversation

dhgwilliam
Copy link
Contributor

this is very much a WIP. feedback welcome. i'm just about to dive in to the types/providers for config. i think i can probably borrow quite a bit of code.

i've also made some presumably debatable decisions which might need to be modified slightly, especially around service management for sensu-server and sensu-api.

i'm smoke testing on centos 6.6, but all the existing (and new) unit tests pass, including debian ones...

fail("Use of private class ${name} by ${caller_module_name}")
}

if $sensu::purge_config and !$sensu::enterprise_dashboard {
Copy link
Contributor

Choose a reason for hiding this comment

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

Likely #401 will end up being merged which changes the purge options (just a heads up)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

p. sure i just rebased against this PR, will update accordingly

@jlambert121
Copy link
Contributor

This is great - thanks for your work! I'd been wondering how easy it would be to support enterprise with this, but not being an enterprise customer I hadn't looked into it at all. I made some comments in the code with thoughts I had as I read over it and would love to see some more testing to go down all of the code paths/options with the changes.

@dhgwilliam dhgwilliam force-pushed the sensu_enterprise branch 2 times, most recently from 475d7c0 to e2cd367 Compare August 11, 2015 18:48
@@ -37,5 +37,10 @@
hasrestart => $hasrestart,
subscribe => [ Class['sensu::package'], Class['sensu::api::config'], Class['sensu::redis::config'] ],
}
} elsif $sensu::manage_services and $::sensu::enterprise {
Copy link
Contributor

Choose a reason for hiding this comment

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

What about something more like this so we don't declare the sensu-api service twice?

And just to confirm, in SE api should not be running correct?

if $sensu::manage_services {

    if $sensu::enterprise {
        $ensure = 'stopped'
        $enable = false
    } elsif $sensu::api {
        $ensure = 'running'
        $enable = true
    } else {
        $ensure = 'stopped'
        $enable = false
    }

    service { 'sensu-api':
      ensure     => $ensure,
      enable     => $enable,
      hasrestart => $hasrestart,
      subscribe  => [ Class['sensu::package'], Class['sensu::api::config'], Class['sensu::redis::config'] ],
    }
  }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, the sensu-enterprise service replaces sensu-api and sensu-server.

Upon reflection, there's actually a gate in init.pp that will fail() if $api and $enterprise are both true, so I think we can simplify this back to the original?

  if $sensu::manage_services {

    case $sensu::api {
      true: {
        $ensure = 'running'
        $enable = true
      }
      default: {
        $ensure = 'stopped'
        $enable = false
      }
    }

...

should actually work as intended!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

indeed, unit tests and acceptance tests pass!

jlambert121 added a commit that referenced this pull request Aug 24, 2015
[WIP] Sensu Enterprise & Enterprise Dashboard support
@jlambert121 jlambert121 merged commit d1819aa into sensu:master Aug 24, 2015
@jlambert121
Copy link
Contributor

Thanks for your effort on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants