Skip to content
This repository has been archived by the owner. It is now read-only.

[dev.icinga.com #3961] Services with no host mappings are not accepted #1256

Closed
icinga-migration opened this issue Apr 12, 2013 · 10 comments
Closed
Milestone

Comments

@icinga-migration
Copy link
Member

@icinga-migration icinga-migration commented Apr 12, 2013

This issue has been migrated from Redmine: https://dev.icinga.com/issues/3961

Created by viranch on 2013-04-12 13:53:33 +00:00

Assignee: mfriedrich
Status: Resolved (closed on 2013-06-22 16:44:08 +00:00)
Target Version: 1.10
Last Update: 2013-06-22 16:44:08 +00:00 (in Redmine)

Icinga Version: 1.8.4
OS Version: CentOS release 6.2 (Final)

dummy config:

define hostgroup {
hostgroup_name grpA
}

define hostgroup {
hostgroup_name grpB
}

define host {
host_name foo
hostgroup grpA,grpB
}

define service {
hostgroup_name grpA,!grpB
}

this fails with the error: "Could not expand hostgroups and/or hosts specified in service" although services with no host mappings should be accepted (and simply ignored). however, the error does not happen if I have a service assigned to an empty hostgroup:

define hostgroup {
hostgroup_name grpC
}
define service {
hostgroup_name grpC
}

// above config is fine for icinga

attaching patch for a possible fix.

Attachments

  • patch.diff viranch - 2013-04-12 13:53:33 +00:00
  • patch2.diff viranch - 2013-04-18 14:14:23 +00:00 - note: this patch is inclusive of the patch for this bug report (because they are over-lapping).

Changesets

2013-06-22 16:11:08 +00:00 by (unknown) 347c328

core: fix services with no host mappings are not accepted (Viranch Metha)

added a test config, verified patch working.
added to THANKS and AUTHORS, preserving full kudos.

fixes #3961
@icinga-migration
Copy link
Member Author

@icinga-migration icinga-migration commented Apr 12, 2013

Updated by mfriedrich on 2013-04-12 16:56:26 +00:00

by host mapping, you mean "grpA,!grpB" would create an empty value (no match) as foo is assigned to both hostgroups, right?

@icinga-migration
Copy link
Member Author

@icinga-migration icinga-migration commented Apr 12, 2013

Updated by viranch on 2013-04-12 19:54:13 +00:00

yes, grpA would add foo and !grpB will discard it back, resulting in empty value (no match) as you said - which is okay and should not result in an error IMO.

@icinga-migration
Copy link
Member Author

@icinga-migration icinga-migration commented Apr 13, 2013

Updated by mfriedrich on 2013-04-13 13:51:00 +00:00

ok, sounds logical, though it must be tested properly in order not break with other stuff. i might reschedule that for an 1.9.1 if there's no time left for 1.9 - thanks for the patch.

@icinga-migration
Copy link
Member Author

@icinga-migration icinga-migration commented Apr 15, 2013

Updated by viranch on 2013-04-15 09:16:06 +00:00

JFYI a short explanation of the patch in case needed by anyone:

prior to the patch-
the 'expand_hostgroups_and_hosts' method returns a expanded memberlist, if this memberlist is NULL, it is assumed that there has been a configuration error (syntax error in host/hostgroup definition, etc), which is wrong because the memberlist is NULL even when it is simply empty and there is no configuration error.

what the patch does-
the expand_hostgroups_and_hosts method is changed to put the resultant memberlist in its first argument and return a return-code (ERROR/OK/etc). so when the memberlist is empty the first argument will still be made NULL, but a OK will be returned to differentiate between a config error and an empty memberlist. all calls to this method have been fixed to follow the new method template and handle the return codes (and NULL memberlists wherever applicable). the real fix is inside duplicate_services method where the return code and empty memberlist (returned by expand_hostgroups_and_hosts) are handled separately to fix the behavior related to this bug report (ERROR return code gives a config error and NULL memberlist simply ignores the service definition); OTOH both are handled as one (ERROR return code or NULL memberlist means config error) elsewhere to keep the behavior consistent with what whatever it is prior to the patch.

EDIT: another FYI- the patch is against the current master.

@icinga-migration
Copy link
Member Author

@icinga-migration icinga-migration commented Apr 18, 2013

Updated by viranch on 2013-04-18 14:14:23 +00:00

  • File added patch2.diff

there is another (similar) bug. example config:

define hostgroup {
  hostgroup_name myGrp
}
define host {
host_name foo
hostgroup myGrp
}
define host {
host_name bar
hostgroup myGrp
}
define service {
hostgroup_name myGrp
host_name "bar"
}
define servicegroup {
servicegroup_name myservices
}

this gives the error: Could not expand member services specified in servicegroup

although it accepts the empty service (grpA, minus foo, minus bar = empty), it generates error when parsing service groups.

what internally happens is when duplicating services, it simply ignores the service (and continues on the for-loop), so the service still remains in the original xodtemplate_service_list, which creates problems in recombobulate servicegroups. so the service should ideally be removed from the list.

I have attached a patch to fix this. this patch is inclusive of the patch for this bug report (because they are over-lapping).

@icinga-migration
Copy link
Member Author

@icinga-migration icinga-migration commented Apr 21, 2013

Updated by mfriedrich on 2013-04-21 10:11:55 +00:00

  • Target Version set to 1.10
@icinga-migration
Copy link
Member Author

@icinga-migration icinga-migration commented Apr 21, 2013

Updated by mfriedrich on 2013-04-21 10:12:07 +00:00

  • Status changed from New to Assigned
  • Assigned to set to mfriedrich
@icinga-migration
Copy link
Member Author

@icinga-migration icinga-migration commented Apr 21, 2013

Updated by mfriedrich on 2013-04-21 10:12:47 +00:00

i do not have time anymore before 1.9 happens, but we may fix it properly in next for 1.10 and backport into 1.9.x then.

@icinga-migration
Copy link
Member Author

@icinga-migration icinga-migration commented Jun 22, 2013

Updated by mfriedrich on 2013-06-22 16:13:44 +00:00

sorry for the delay. applied the patch, added a modified test config into tests/etc/3961.cfg - reproducable output, and none with the patch applied.

thanks for your contribution!

@icinga-migration
Copy link
Member Author

@icinga-migration icinga-migration commented Jun 22, 2013

Updated by Anonymous on 2013-06-22 16:44:08 +00:00

  • Status changed from Assigned to Resolved
  • Done % changed from 0 to 100

Applied in changeset 347c328.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.