Skip to content

Conversation

@tomas-edwardsson
Copy link
Contributor

What it does

Enables template options via --opt VAR=SOMETHING

What it does NOT

Break how everything used to work, so old templates still work

Description

$ okconfig addtemplate www.okconfig.org --template http \
        --opt VIRTUAL_HOST=vhost.okconfig.org \
        --opt SEARCH_STRING=something
Saved /etc/nagios/okconfig/hosts/default/www.okconfig.org-http-vhost.okconfig.org.cfg

Contents of the file (/etc/nagios/okconfig/hosts/default/www.okconfig.org-http-vhost.okconfig.org.cfg) become

define service {
    use         okc-check_http
    host_name       www.okconfig.org
    contact_groups      default
    service_description HTTP vhost.okconfig.org
    #check_command      okc-check_http

    __VIRTUAL_HOST      vhost.okconfig.org
    #__URI          /
    __SEARCH_STRING something
    #__RESPONSE_WARNING 2.0
    #__RESPONSE_CRITICAL    10.0
    #__PORT         80
    #__ON_REDIRECT           follow

}

Then there is the configuration for the template options.
/usr/share/okconfig/examples/http.yml

filename:   "{HOSTNAME}-http-{VIRTUAL_HOST}.cfg"
parms:
    VIRTUAL_HOST: 
        default:    k:HOSTNAME
        summary:    Virtual host to query for
    URI:
        default:    /
    SEARCH_STRING:
        default:    
    RESPONSE_WARNING:
        default:    2.0
        summary:    Response time in seconds till warning
    RESPONSE_CRITICAL:
        default:    10.0
        summary:    Response time in seconds till critical
    PORT:
        default:    80
    ON_REDIRECT:
        default:    follow
        summary:    Action to on redirects, eg <ok|warning|critcal|follow|sticky|stickyport>

It still uses the example file which now looks like this:

define service {
    use         okc-check_http
    host_name       HOSTNAME
    contact_groups      GROUP
    service_description HTTP ^VIRTUAL_HOST
    #check_command      okc-check_http

    __VIRTUAL_HOST      ^VIRTUAL_HOST
    #__URI          ^URI
    #__SEARCH_STRING    ^SEARCH_STRING
    #__RESPONSE_WARNING ^RESPONSE_WARNING
    #__RESPONSE_CRITICAL    ^RESPONSE_CRITICAL
    #__PORT         ^PORT
    #__ON_REDIRECT           follow

}

@coveralls
Copy link

Coverage Status

Coverage increased (+2.6%) when pulling 1b1f3b3 on templateopts into dff24b1 on master.

@palli
Copy link
Contributor

palli commented Jun 28, 2014

OK, starting to look at this now. Dumping questions as i see them.

Is the feature not more appropriately named template variables ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Please move this import to the top of the file.

Also, please put yaml dependency in requirements.txt, rpm spec file and debian/ directory

@palli
Copy link
Contributor

palli commented Jul 2, 2014

Ok i didnt run this yet, but the code diff looks good.

Then i just stumbled on to an idea that might seem idiodic but...

Seems like in your http example there is 1:1 mapping between attributes, and the "template option names".

Why not have this --opt foo=bar describe any attribute name ? Benefits:

  • We don't have to remove all default values from the example files (maintains cleanliness)
  • We support changing any attribute, not just the options that we provide
  • We don't need the extra yaml file

Cons:

  • We haven't solved the "cannot add the same template twice" problem, but there are other ways to work around that (like providing --destination-filename paramater, or not overwrite the file by default)

Again, we come back to we need more concrete examples. Currently the http feature is kind of already implemented in adagios via okconfig > add service

@palli
Copy link
Contributor

palli commented Jul 2, 2014

(so a quick summary, i dont see a problem with the code per se, but i worry that the feature could be implemented without a need for another config file.

@tomas-edwardsson
Copy link
Contributor Author

The destination-filename is a stumbling block, plus alternate service_descriptions which I want to address. Instead of going with the mssql, I'm going to post an example using check_postgres which highlights the problem more adequetaly. There we have server based checks and then we have per-database checks. The per database checks would need to be noted in the service_description and I would see adding a postgres-{hostname}-{database}.cfg for each database.

Why a configuration file? I want to enrich the user experience of arguments where there is a description of what the option is and even going with enum fields where there are only a few valid options. No functionality has been implemented yet but consumers can read the description field and I'm working on the functionality of okconfig addtemplate -i <template_name> which will invoke a interactive mode and allow you via command line to set template variables.

The 1:1 mapping is almost correct but I'm offering for sane defaults where something like VIRTUAL_HOST can be derived from HOSTNAME by default but the user is allowed to specify his own VIRTUAL_HOST where appropriate.

@palli
Copy link
Contributor

palli commented Jul 2, 2014

Ok sounds good. Postgres template sounds nice.

@palli
Copy link
Contributor

palli commented Jul 3, 2014

Additionally, thoughts on a less confusing name ? (suggestion: template variables)

@pall-valmundsson
Copy link
Contributor

I have a few templates that would benefit from this. Basically all checks that belong on a host that have to check via some 3rd party, e.g. iLO checks need an IP address parameter and vSphere node checks need various parameters for querying the vSphere API.

I like the "template variables" name better as well.

@palli
Copy link
Contributor

palli commented Nov 16, 2014

Are you still working on this ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like yaml is a new dependency. Should be marked as such in packaging.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 13 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

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.

5 participants