okconfig icon indicating copy to clipboard operation
okconfig copied to clipboard

Enable template options

Open tomas-edwardsson opened this issue 11 years ago • 10 comments

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

}

tomas-edwardsson avatar Jun 12 '14 13:06 tomas-edwardsson

Coverage Status

Coverage increased (+2.6%) when pulling 1b1f3b312cfab44313aae308f052642775937019 on templateopts into dff24b1fcf797e753f8dca639035ea0428072062 on master.

coveralls avatar Jun 12 '14 13:06 coveralls

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

Is the feature not more appropriately named template variables ?

palli avatar Jun 28 '14 13:06 palli

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 avatar Jul 02 '14 17:07 palli

(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.

palli avatar Jul 02 '14 17:07 palli

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.

tomas-edwardsson avatar Jul 02 '14 20:07 tomas-edwardsson

Ok sounds good. Postgres template sounds nice.

palli avatar Jul 02 '14 22:07 palli

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

palli avatar Jul 03 '14 16:07 palli

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.

pall-valmundsson avatar Sep 02 '14 22:09 pall-valmundsson

Are you still working on this ?

palli avatar Nov 16 '14 14:11 palli

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

sonarqubecloud[bot] avatar Nov 26 '21 13:11 sonarqubecloud[bot]