opinkerfi / adagios

Adagios - Web Based Nagios Configuration
GNU Affero General Public License v3.0
327 stars 75 forks source link

Default host/service/contact creation breaks default configuration #527

Closed ghomem closed 9 years ago

ghomem commented 9 years ago

I tried to define a new service based on an existing check command. During the service definition I only changed the checkcommand and saved, leaving everything else as it was. The configuration couldn't be applied. The following error could be seen by running

nagios -v /etc/nagios/nagios.cfg:

Error: Invalid max_attempts, check_interval, retry_interval, or notification_interval value for service 'Load check via SSH' on host

The ofending definition was

# Configuration file /etc/nagios/okconfig/hosts/default/webserver01.angulosolido.pt-host.cfg # Edited by PyNag on Mon Feb 16 18:09:11 2015

define service {
         service_description            CPU load
         host_name                      XXX.YYY.ZZZ
        check_command                 ssh_load!22!1.5,1.25,1!2,1.75,1.5!
}

Suggestion: all the parameters which are necessary to not break the configuration should either have defaults or be mandatory for the user to fill, before the configuration is saved.

UPDATE: I updated the title because the problem is the same for services, hosts and contacts. The used template which is present in the creation step in the GUI (generic-service, generic-host, generic-contact) does not get reflected in the configuration. See https://github.com/opinkerfi/adagios/issues/527#issuecomment-74664598

ghomem commented 9 years ago

If we add this manually then it works:

   max_check_attempts         3 
    normal_check_interval      10 
    retry_check_interval          2 
    notification_interval           60 

but this was manually copied from generic-service from where the new service should have inherited automatically.

ghomem commented 9 years ago

If we change the default inheritance in the GUI from generic-service to local-service then it works once we set the check command. All defaults are filled. Any comment?

ghomem commented 9 years ago

In the first case Pynag forgets to add use generic-service to the definition. In the second one use local-service is there.

ghomem commented 9 years ago

I understood the problem. The inheritance from generic-service is lost from the first step to the second, whereas the inheritance from local-service is not. The same thing happens for hosts and contacts.

Can I help getting this fixed? Any hint on where to start?

generic1 generic2 local1 local2

mayasd commented 9 years ago

AddObjectForm use save() method of PynagForm. This method call changed_data() to filter only data who has changed. But use field is initialized in AddObjectForm and can't appear like a changed data.

Solution will be maybe to modify save() method to be different when you add object or edit object ? I can help on this if needed ;)

ghomem commented 9 years ago

I agree. It doesn't make sense to present only the data that has changed because inheritance is essential to avoid breaking the config. I'd be willing to test a patched file if you send me one.

ghomem commented 9 years ago

Question: is this problem only happening to me?

It would seem that Adagios is mostly unusable as a GUI if you can't add an object without risking to break the configuration in case you don't re-enter parameters that were initially present as defaults.

But it could be that I am misunderstanding something. I'm debugging the save function now.

ghomem commented 9 years ago

Here is a workaround.

On /usr/lib/python2.6/site-packages/adagiosobjectbrowser/forms.py edit the save method for class PynagForm by adding:

# https://github.com/opinkerfi/adagios/issues/527
value = self.cleaned_data['use']
self.pynag_object['use'] = value

just before

self.pynag_object.save()
ghomem commented 9 years ago

Notes to whoever is interested:

1- a call to function add_object present in objectbrowser/views.py is triggered when browser issues GET request to https://NAGIOSURL/adagios/objectbrowser/add/TYPE 2- an initial object of type AddObjectForm is created with empty values except for the 'use' field which by default contains generic-TYPE (generic-host, generic-service,...) 3- user fills the parameters in the browser and presses the submit button 4- brower issues a POST to the same page https://NAGIOSURL/adagios/objectbrowser/add/TYPE (form action="#" in the html files) 5- POST data is correct, contains the 'use' field 6- function add_object is called again doing different work (it tests for GET and POST and behaves accordingly); a new AddObjectForm is created from the POST data 7- the save method for AddObjectForm is called and saves INCOMPLETE DATA (the 'use' field is lost) 8- we are redirected to https://NAGIOSURL/adagios/objectbrowser/edit/-120252901566303994 where we see the INCOMPLETE DATA saved at step 7

The save method saves incomplete data because it is looking for the fields that were changed by the user in relation to the initial data (visible at step 2). Since the initial state was never saved, all the initial data (which is only the 'use' field but...) is lost in the process. What the save method should be doing is saving all the data present in the form regardless of it having been changed or filled by the user of not.

Class inheritance:

forms.Form -> AdagiosForm -> PynagForm -> AddObjectForm

palli commented 9 years ago

Thanks for the detailed analysis ghomem and sorry for not seeing this earlier.

Can you confirm if latest master fixes this issue ?

ghomem commented 9 years ago

Hi,

Is it enough to get adagios/objectbrowser/forms.py ?

I used your great "Installing on redhat based systems" instructions for the system I am testing.

Let me know which is the best way to test. I will provide feedback.

Many thank! Gustavo

----- Original Message -----

From: "Pall Sigurdsson" notifications@github.com To: "opinkerfi/adagios" adagios@noreply.github.com Cc: "ghomem" gustavo@angulosolido.pt Sent: Saturday, April 4, 2015 6:39:40 PM Subject: Re: [adagios] Default host/service/contact creation breaks default configuration (#527)

Thanks for the detailed analysis ghomem and sorry for not seeing this earlier.

Can you confirm if latest master fixes this issue ?


Reply to this email directly or view it on GitHub: https://github.com/opinkerfi/adagios/issues/527#issuecomment-89624914

tomas-edwardsson commented 9 years ago

You should be able to update from the latest code using the following:

yum --enablerepo=ok-testing clean metadata
yum --enablerepo=ok-testing update pynag adagios
service httpd restart
ghomem commented 9 years ago

@Tomas Edwardsson

Thanks, that worked.

@palli

The 'use' template inheritance problem seems to be fixed at

adagios 1.6.1-1.git.143.fc57957.el6 pynag 0.9.1-1.git.165.9b69b4f.el6

However when one creates a new service it inherits from generic-service which does not have a default check_command. If one forgets to add it the configuration goes broken and there is now way to fix it via GUI (broken service is created but invisible via GUI before reload, reload fails because service has no check_command)

Solution: the initial service creation form should have a visible and mandatory check_command field. This is quite logical because check_command is the central entity of a service. It should not be relegated to a secondary form.

I'm available for further testing.